[ 
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)

Reply via email to