[ https://issues.apache.org/jira/browse/CASSANDRA-10740?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15021845#comment-15021845 ]
Jakub Janecek commented on CASSANDRA-10740: ------------------------------------------- I did but it seemed like too heavy-weight approach. The algorithm is quite easy to test and I would like to keep it that way. Actually without the necessity to have method SSTableReader.firstKeyBeyond I could refactor the algorithmic part into a separate method which could be tested and let the SSTableReader dependency in the current method. But ok... can you point me to some actual test I can "copy" to start? > Incorrect condition causes cleanup of SSTables which might not need it > ---------------------------------------------------------------------- > > Key: CASSANDRA-10740 > URL: https://issues.apache.org/jira/browse/CASSANDRA-10740 > Project: Cassandra > Issue Type: Bug > Reporter: Jakub Janecek > Fix For: 2.1.x > > Attachments: trunk-10740.txt > > > We needed to perform cleanup of SSTables in our production cluster but found > out that it would take several months. Together with my colleague > [~JiriHorky] we have pinpointed the issue to method > CompactionManager.needsCleanup which incorrectly marks almost all SSTables as > the ones needing the cleanup even though that might not be true. I am > attaching a patch which should be applicable from version 2.x forward (this > is the version we would need it in). > To explain the problem, the last condition in the cycle that checks whether > the next key in the SSTable after the current checked range is NOT contained > in the next range falsely marks SSTable as necessary to have cleanup even > though it might be contained in some other range. The correct condition > (which is actully described in the comment) is to check whether the next key > in the SSTable after the current range is less than or equal to the "left" > key of the next range (assuming that the left key is exclusive which is > described in another comment of the same method). > Is our patch and reasoning correct? -- This message was sent by Atlassian JIRA (v6.3.4#6332)