cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Benedict (JIRA)" <j...@apache.org>
Subject [jira] [Comment Edited] (CASSANDRA-10202) simplify CommitLogSegmentManager
Date Mon, 20 Jun 2016 20:40:58 GMT

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

Benedict edited comment on CASSANDRA-10202 at 6/20/16 8:40 PM:
---------------------------------------------------------------

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) en route fewer 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).


was (Author: benedict):
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)

Mime
View raw message