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

Alex Petrov edited comment on CASSANDRA-12956 at 12/1/16 8:22 PM:
------------------------------------------------------------------

Thanks for the review!

I was also skeptical about two threads waiting for one another. Also, tried the 
approach you've suggested.
I hesitated mostly because it'd be blocking the flush thread (although you're 
right that it's going to be
waiting for flushes anyways) and because {{flushMemtable}} is called from loop, 
so I wasn't sure if it's
a good place.

In retrospect, I think your suggestion is much better than the previous 
version. I've re-implemented a patch
for 3.0 as well, it got much simpler. Now, we do 2i flush before memtable flush 
during flush of "data" memtable
(first one). We could bring back changes that expose sstable writer and run 2i 
flush on the other
thread and commit sstable only on successful 2i flush, although since all cf 
memtables are flushed sequentially
anyways and it might be a bit out of scope of the bugfix I decided to leave it 
this way. Simply running 2i
flush in a different thread is not enough, as we need to ensure it's in sync 
with "data" memtable flush.

Order of 2i/memtable flush does not matter, as for 2i it's only important that 
data is present either in
sstable or in memtable. We can have the following situations: flush running 
(memtable is queried), flush
successfull (sstable is queried), flush unsuccessful (memtable is queried), 
flush unsuccessful + node restarted
(CL will replay the data and it'll be available from memtable again). So 3.0 
patch relies on this behaviour.

|[3.0|https://github.com/ifesdjeen/cassandra/tree/12956-3.0-v2]|[utest|https://cassci.datastax.com/view/Dev/view/ifesdjeen/job/ifesdjeen-12956-3.0-v2-testall/]|[dtest|https://cassci.datastax.com/view/Dev/view/ifesdjeen/job/ifesdjeen-12956-3.0-v2-dtest/]|
|[3.X|https://github.com/ifesdjeen/cassandra/tree/12956-3.X]|[utest|https://cassci.datastax.com/view/Dev/view/ifesdjeen/job/ifesdjeen-12956-3.X-testall/]|[dtest|https://cassci.datastax.com/view/Dev/view/ifesdjeen/job/ifesdjeen-12956-3.X-dtest/]|
|[trunk|https://github.com/ifesdjeen/cassandra/tree/12956-trunk]|[utest|https://cassci.datastax.com/view/Dev/view/ifesdjeen/job/ifesdjeen-12956-trunk-testall/]|[dtest|https://cassci.datastax.com/view/Dev/view/ifesdjeen/job/ifesdjeen-12956-trunk-dtest/]|



was (Author: ifesdjeen):
Thanks for the review!

I was also skeptical about two threads waiting for one another. Also, tried the 
approach you've suggested.
I hesitated mostly because it'd be blocking the flush thread (although you're 
right that it's going to be
waiting for flushes anyways) and because {{flushMemtable}} is called from loop, 
so I wasn't sure if it's
a good place.

In retrospect, I think your suggestion is much better than the previous 
version. I've re-implemented a patch
for 3.0 as well, it got much simpler. Now, we do 2i flush before memtable flush 
during flush of "data" memtable
(first one). We could bring back changes that expose sstable writer and run 2i 
flush on the other
thread and commit sstable only on successful 2i flush, although since all cf 
memtables are flushed sequentially
anyways and it might be a bit out of scope of the bugfix I decided to leave it 
this way. Simply running 2i
flush in a different thread is not enough, as we need to ensure it's in sync 
with "data" memtable flush.

Order of 2i/memtable flush does not matter, as for 2i it's only important that 
data is present either in
sstable or in memtable. We can have the following situations: flush running 
(memtable is queried), flush
successfull (sstable is queried), flush unsuccessful (memtable is queried), 
flush unsuccessful + node restarted
(CL will replay the data and it'll be available from memtable again). So 3.0 
patch relies on this behaviour.

|[3.0-v2|https://github.com/ifesdjeen/cassandra/tree/12956-3.0-v2-v2]|[utest|https://cassci.datastax.com/view/Dev/view/ifesdjeen/job/ifesdjeen-12956-3.0-v2-v2-testall/]|[dtest|https://cassci.datastax.com/view/Dev/view/ifesdjeen/job/ifesdjeen-12956-3.0-v2-v2-dtest/]|
|[3.X|https://github.com/ifesdjeen/cassandra/tree/12956-3.X]|[utest|https://cassci.datastax.com/view/Dev/view/ifesdjeen/job/ifesdjeen-12956-3.X-testall/]|[dtest|https://cassci.datastax.com/view/Dev/view/ifesdjeen/job/ifesdjeen-12956-3.X-dtest/]|
|[trunk|https://github.com/ifesdjeen/cassandra/tree/12956-trunk]|[utest|https://cassci.datastax.com/view/Dev/view/ifesdjeen/job/ifesdjeen-12956-trunk-testall/]|[dtest|https://cassci.datastax.com/view/Dev/view/ifesdjeen/job/ifesdjeen-12956-trunk-dtest/]|


> CL is not replayed on custom 2i exception
> -----------------------------------------
>
>                 Key: CASSANDRA-12956
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-12956
>             Project: Cassandra
>          Issue Type: Bug
>            Reporter: Alex Petrov
>            Assignee: Alex Petrov
>            Priority: Critical
>
> If during the node shutdown / drain the custom (non-cf) 2i throws an 
> exception, CommitLog will get correctly preserved (segments won't get 
> discarded because segment tracking is correct). 
> However, when it gets replayed on node startup,  we're making a decision 
> whether or not to replay the commit log. CL segment starts getting replayed, 
> since there are non-discarded segments and during this process we're checking 
> whether there every [individual 
> mutation|https://github.com/apache/cassandra/blob/trunk/src/java/org/apache/cassandra/db/commitlog/CommitLogReplayer.java#L215]
>  in commit log is already committed or no. Information about the sstables is 
> taken from [live sstables on 
> disk|https://github.com/apache/cassandra/blob/trunk/src/java/org/apache/cassandra/db/commitlog/CommitLogReplayer.java#L250-L256].



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

Reply via email to