[jira] [Commented] (CASSANDRA-8429) Some keys unreadable during compaction

["Benedict (JIRA)" <ji...@apache.org>]

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

Benedict commented on CASSANDRA-8429:
-------------------------------------

Apologies for the delay on this one. I've had a bit of fun digging through it and, in the end, cleaning up some of the inadequate decisions I made first time around so that this code is hopefully easier for everyone to read and more obviously correct (I was having trouble convincing myself prior).

Mostly I've been trying to figure out what on earth was changed for this patch to introduce an infrequent but persistent problem with reference counts. However after reslicing the code I have tracked it down to an unrelated already present bug that I will file and fix separately, that was clearly just being made more likely with the change, timing-wise. The positive upshot, though, is that I have made the code this patch is based on quite a bit easier to read, and I hope more clearly correct as a result. I've uploaded [here|https://github.com/belliottsmith/cassandra/commits/8429].

The patch is split into 5 commits:
# Improves legibility by removing Pair and replacing with a bespoke class
# Merges all of the new methods introduced in this (and related) patches to an enum flag provided to a single method (in each necessary class). Also fixes a minor memory leak introduced here for compression metadata.
# Fixes an unrelated bug that was somehow being exercised here and preventing a clean run of unit tests
# Refactors the abort() and finish() methods in SSTableRewriter to make them more legible and also make the cleanup more obviously correct. This is the only complex review to do, though it's not too bad:
#* The finishedEarly and finishedWriters collections are merged, and a special discard collection is introduced to take the place of the former _only_ when the sstable has a replacement. On closing a writer we immediately remove it from finishedEarly and move its most recent reader to discard.
#* The cleanup of discardable readers has been split out into a shared method used by both abort() and finish() 
#* finish() and finishAndThrow() have been merged so that they're obviously the same
#* Instead of managing the Iterator ourselves, I've wrapped it in an Iterator that removes as we progress, so the complex methods have less boilerplate
# Removes the use of sharesBfWith() as setReplacedBy() is both more correct and easier to reason about (sharesBfWith could result in a leak or a memory bug if sstables are released out of order), also fixes the deletion logic on cleanup that may explain why this wasn't used previously

We can probably split some of these commits into another ticket if you prefer, but I'd feel most comfortable committing all of them together. I've filed separate tickets for all of the other issues I came across while reviewing that are easily delayed or done separately.

> Some keys unreadable during compaction
> --------------------------------------
>
>                 Key: CASSANDRA-8429
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-8429
>             Project: Cassandra
>          Issue Type: Bug
>         Environment: Ubuntu 14.04
>            Reporter: Ariel Weisberg
>            Assignee: Marcus Eriksson
>             Fix For: 2.1.3
>
>         Attachments: cluster.conf, run_stress.sh
>
>
> Starts as part of merge commit 25be46497a8df46f05ffa102bc645bfd684ea48a
> Stress will say that a key wasn't validated because it isn't returned even though it's loaded. The key will eventually appear and can be queried using cqlsh.
> Reproduce with
> #!/bin/sh
> ROWCOUNT=10000000
> SCHEMA='-col n=fixed(1) -schema compaction(strategy=LeveledCompactionStrategy) compression=LZ4Compressor'
> ./cassandra-stress write n=$ROWCOUNT -node xh61 -pop seq=1..$ROWCOUNT no-wrap -rate threads=25 $SCHEMA
> ./cassandra-stress mixed "ratio(read=2)" n=100000000 -node xh61 -pop "dist=extreme(1..$ROWCOUNT,0.6)" -rate threads=25 $SCHEMA



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