Mailing-List: contact commits-help@cassandra.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@cassandra.apache.org
Date: Mon, 20 Jun 2016 20:39:58 +0000 (UTC)
From: "Benedict (JIRA)" <jira@apache.org>
To: commits@cassandra.apache.org
Message-ID: <JIRA.12859199.1440606133000.2338.1466455198007@Atlassian.JIRA>
In-Reply-To: <JIRA.12859199.1440606133000@Atlassian.JIRA>
References: <JIRA.12859199.1440606133000@Atlassian.JIRA> <JIRA.12859199.1440606133353@arcas>
Subject: [jira] [Commented] (CASSANDRA-10202) simplify
 CommitLogSegmentManager
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
archived-at: Mon, 20 Jun 2016 20:39:59 -0000


    [ https://issues.apache.org/jira/browse/CASSANDRA-10202?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15340354#comment-15340354 ] 

Benedict commented on CASSANDRA-10202:
--------------------------------------

We also ideally want lock-free swapping-in of the new segment, no?  Currently we don't have it, but until we reach _pure_-TPC (probably never) fen route ewer application threads exposes us to a higher risk of gumming up the system.

But yes, we could do full mutex, but it is still significantly safer to move it all into one structure where that is well managed.  The prior art has it clumsily littered amongst all the other code.  Thing is, once you do that you essentially have the new algorithm, just with one of the methods wrapped in an unnecessary mutex call.

I do agree the code should be tested better, but that is true of everything - the current code is trusted only on the word of commitlog-stress, making this as trustworthy, but it is always better to improve that.  

I would however reiterate I don't necessarily think the patch entirely warrants inclusion, I just want the discussion to be a bit more informed.

On the topic of generic linked-lists, I have two view points: 1) I've attempted to integrate any number of generic linked-lists, and they are universally rejected\*, so I gave up and tried to stick to hyper-safety-oriented structures that have functionality hamstrung as far as possible in light of the use case constraints; 2) those constraints matter for readability and function, too, and you can end up with a more powerful linked-list for your situation despite a less powerful overall structure, as well as one that tells you more about the behaviour of its users.

I'd point out that this whole code area is massively concurrent, as is the whole project.  This linked-list is by far the easiest part of this code, and most of the project, to reason about concurrency-wise.  If we do not trust ourselves to write it, we should probably start introspecting about what that means.

NB: I must admit I haven't read the code in question for a while, and am typing this all from memory, in bed recovering from flu, so I might just be delirious.  It could all be terrible.

\* Notably, I can recall at least two serious bugs that would have been avoided with one of these structures has they been included when proferred. One occurred in this code, the other was down to a pathological and unexpected behaviour in ConcurrentLinkedQueue, the most battle-tested structure around (it was an understood behaviour by the author, just undocumented and unexpected).

> simplify CommitLogSegmentManager
> --------------------------------
>
>                 Key: CASSANDRA-10202
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-10202
>             Project: Cassandra
>          Issue Type: Improvement
>          Components: Local Write-Read Paths
>            Reporter: Jonathan Ellis
>            Assignee: Branimir Lambov
>            Priority: Minor
>
> Now that we only keep one active segment around we can simplify this from the old recycling design.


--
This message was sent by Atlassian JIRA
(v6.3.4#6332)