[jira] [Commented] (CASSANDRA-8683) Incremental repairs broken with early opening of compaction results
[
https://issues.apache.org/jira/browse/CASSANDRA-8683?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14293426#comment-14293426
]
Benedict commented on CASSANDRA-8683:
-
I am struggling to reproduce this test locally, but by spinning on it I have
produced:
{noformat}
[junit] Testcase:
testValidationMultipleSSTablePerLevel(org.apache.cassandra.db.compaction.LeveledCompactionStrategyTest):
Caused an ERROR
[junit] java.lang.AssertionError: Memory was freed
[junit] java.util.concurrent.ExecutionException: java.lang.AssertionError:
Memory was freed
[junit] at java.util.concurrent.FutureTask.report(FutureTask.java:122)
[junit] at java.util.concurrent.FutureTask.get(FutureTask.java:188)
[junit] at
org.apache.cassandra.db.compaction.LeveledCompactionStrategyTest.testValidationMultipleSSTablePerLevel(LeveledCompactionStrategyTest.java:184)
[junit] Caused by: java.lang.AssertionError: Memory was freed
[junit] at
org.apache.cassandra.io.util.Memory.checkPosition(Memory.java:281)
[junit] at org.apache.cassandra.io.util.Memory.getInt(Memory.java:233)
[junit] at
org.apache.cassandra.io.sstable.IndexSummary.getPositionInSummary(IndexSummary.java:118)
[junit] at
org.apache.cassandra.io.sstable.IndexSummary.getKey(IndexSummary.java:123)
[junit] at
org.apache.cassandra.io.sstable.format.SSTableReader$9$1.next(SSTableReader.java:1249)
[junit] at
org.apache.cassandra.io.sstable.format.SSTableReader$9$1.next(SSTableReader.java:1226)
[junit] at
com.google.common.collect.Iterators$5.next(Iterators.java:553)
[junit] at
org.apache.cassandra.repair.Validator.prepare(Validator.java:89)
[junit] at
org.apache.cassandra.db.compaction.CompactionManager.doValidationCompaction(CompactionManager.java:1007)
[junit] at
org.apache.cassandra.db.compaction.CompactionManager.access$600(CompactionManager.java:95)
[junit] at
org.apache.cassandra.db.compaction.CompactionManager$9.call(CompactionManager.java:591)
[junit] at java.util.concurrent.FutureTask.run(FutureTask.java:262)
[junit] at
java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1145)
[junit] at
java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:615)
[junit] at java.lang.Thread.run(Thread.java:745)
{noformat}
Which may or may not be related, but is certainly problematic.
Incremental repairs broken with early opening of compaction results
---
Key: CASSANDRA-8683
URL: https://issues.apache.org/jira/browse/CASSANDRA-8683
Project: Cassandra
Issue Type: Bug
Reporter: Marcus Eriksson
Assignee: Marcus Eriksson
Fix For: 2.1.3
Attachments: 0001-avoid-NPE-in-getPositionsForRanges.patch
Incremental repairs holds a set of the sstables it started the repair on (we
need to know which sstables were actually validated to be able to anticompact
them). This includes any tmplink files that existed when the compaction
started (if we wouldn't include those, we would miss data since we move the
start point of the existing non-tmplink files)
With CASSANDRA-6916 we swap out those instances with new ones
(SSTR.cloneWithNewStart / SSTW.openEarly), meaning that the underlying file
can get deleted even though we hold a reference.
This causes the unit test error:
http://cassci.datastax.com/job/trunk_utest/1330/testReport/junit/org.apache.cassandra.db.compaction/LeveledCompactionStrategyTest/testValidationMultipleSSTablePerLevel/
(note that it only fails on trunk though, in 2.1 we don't hold references to
the repairing files for non-incremental repairs, but the bug should exist in
2.1 as well)
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (CASSANDRA-8683) Incremental repairs broken with early opening of compaction results
[ https://issues.apache.org/jira/browse/CASSANDRA-8683?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14293446#comment-14293446 ] Marcus Eriksson commented on CASSANDRA-8683: When I debugged this, the reason that we fell through all the way to that return null was that the index only contained the last key, then we were at EOF and ended the loop Incremental repairs broken with early opening of compaction results --- Key: CASSANDRA-8683 URL: https://issues.apache.org/jira/browse/CASSANDRA-8683 Project: Cassandra Issue Type: Bug Reporter: Marcus Eriksson Assignee: Marcus Eriksson Fix For: 2.1.3 Attachments: 0001-avoid-NPE-in-getPositionsForRanges.patch Incremental repairs holds a set of the sstables it started the repair on (we need to know which sstables were actually validated to be able to anticompact them). This includes any tmplink files that existed when the compaction started (if we wouldn't include those, we would miss data since we move the start point of the existing non-tmplink files) With CASSANDRA-6916 we swap out those instances with new ones (SSTR.cloneWithNewStart / SSTW.openEarly), meaning that the underlying file can get deleted even though we hold a reference. This causes the unit test error: http://cassci.datastax.com/job/trunk_utest/1330/testReport/junit/org.apache.cassandra.db.compaction/LeveledCompactionStrategyTest/testValidationMultipleSSTablePerLevel/ (note that it only fails on trunk though, in 2.1 we don't hold references to the repairing files for non-incremental repairs, but the bug should exist in 2.1 as well) -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (CASSANDRA-8683) Incremental repairs broken with early opening of compaction results
[ https://issues.apache.org/jira/browse/CASSANDRA-8683?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14293410#comment-14293410 ] Benedict commented on CASSANDRA-8683: - I think this might still not be quite right, but we're on the right track. This also highlights another bug (CASSANDRA-8691). I suspect it may be the mistake that we are passing in .getToken().maxKeyBound() to getPosition - it could be we are looking up a value beyond that present in the index, because there are multiple adjacent keys with the same token, and we are looking past all of them. That said, if we fix CASSANDRA-8691 we will need to do something about the last index position. There are two possibilities: 1) Like suggested a while back, we could simply ignore the last record for purposes of getPositionsForRanges(), since our paired source file will contain the row. Or 2) during index construction we could perhaps retain a lookup of the records immediately following an index boundary, for the past few boundaries only. We could then use this as our last key instead. I'm not sure which I prefer, as 1) creates some risk it will not be accounted for in future; 2) creates some unnecessary complexity. Incremental repairs broken with early opening of compaction results --- Key: CASSANDRA-8683 URL: https://issues.apache.org/jira/browse/CASSANDRA-8683 Project: Cassandra Issue Type: Bug Reporter: Marcus Eriksson Assignee: Marcus Eriksson Fix For: 2.1.3 Attachments: 0001-avoid-NPE-in-getPositionsForRanges.patch Incremental repairs holds a set of the sstables it started the repair on (we need to know which sstables were actually validated to be able to anticompact them). This includes any tmplink files that existed when the compaction started (if we wouldn't include those, we would miss data since we move the start point of the existing non-tmplink files) With CASSANDRA-6916 we swap out those instances with new ones (SSTR.cloneWithNewStart / SSTW.openEarly), meaning that the underlying file can get deleted even though we hold a reference. This causes the unit test error: http://cassci.datastax.com/job/trunk_utest/1330/testReport/junit/org.apache.cassandra.db.compaction/LeveledCompactionStrategyTest/testValidationMultipleSSTablePerLevel/ (note that it only fails on trunk though, in 2.1 we don't hold references to the repairing files for non-incremental repairs, but the bug should exist in 2.1 as well) -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (CASSANDRA-8683) Incremental repairs broken with early opening of compaction results
[ https://issues.apache.org/jira/browse/CASSANDRA-8683?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14293296#comment-14293296 ] Marcus Eriksson commented on CASSANDRA-8683: [~aboudreault] it could explain the exceptions you saw, could you rerun with the patch? Incremental repairs broken with early opening of compaction results --- Key: CASSANDRA-8683 URL: https://issues.apache.org/jira/browse/CASSANDRA-8683 Project: Cassandra Issue Type: Bug Reporter: Marcus Eriksson Assignee: Marcus Eriksson Fix For: 2.1.3 Attachments: 0001-avoid-NPE-in-getPositionsForRanges.patch Incremental repairs holds a set of the sstables it started the repair on (we need to know which sstables were actually validated to be able to anticompact them). This includes any tmplink files that existed when the compaction started (if we wouldn't include those, we would miss data since we move the start point of the existing non-tmplink files) With CASSANDRA-6916 we swap out those instances with new ones (SSTR.cloneWithNewStart / SSTW.openEarly), meaning that the underlying file can get deleted even though we hold a reference. This causes the unit test error: http://cassci.datastax.com/job/trunk_utest/1330/testReport/junit/org.apache.cassandra.db.compaction/LeveledCompactionStrategyTest/testValidationMultipleSSTablePerLevel/ (note that it only fails on trunk though, in 2.1 we don't hold references to the repairing files for non-incremental repairs, but the bug should exist in 2.1 as well) -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (CASSANDRA-8683) Incremental repairs broken with early opening of compaction results
[ https://issues.apache.org/jira/browse/CASSANDRA-8683?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14293452#comment-14293452 ] Benedict commented on CASSANDRA-8683: - But reaching there is a different bug, especially if opened early. We explicitly leave an entire index summary page (and hence effective interval of index) distance between the last token and where we've written to, so that we have room to scan past if necessary. Incremental repairs broken with early opening of compaction results --- Key: CASSANDRA-8683 URL: https://issues.apache.org/jira/browse/CASSANDRA-8683 Project: Cassandra Issue Type: Bug Reporter: Marcus Eriksson Assignee: Marcus Eriksson Fix For: 2.1.3 Attachments: 0001-avoid-NPE-in-getPositionsForRanges.patch Incremental repairs holds a set of the sstables it started the repair on (we need to know which sstables were actually validated to be able to anticompact them). This includes any tmplink files that existed when the compaction started (if we wouldn't include those, we would miss data since we move the start point of the existing non-tmplink files) With CASSANDRA-6916 we swap out those instances with new ones (SSTR.cloneWithNewStart / SSTW.openEarly), meaning that the underlying file can get deleted even though we hold a reference. This causes the unit test error: http://cassci.datastax.com/job/trunk_utest/1330/testReport/junit/org.apache.cassandra.db.compaction/LeveledCompactionStrategyTest/testValidationMultipleSSTablePerLevel/ (note that it only fails on trunk though, in 2.1 we don't hold references to the repairing files for non-incremental repairs, but the bug should exist in 2.1 as well) -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (CASSANDRA-8683) Incremental repairs broken with early opening of compaction results
[
https://issues.apache.org/jira/browse/CASSANDRA-8683?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14293442#comment-14293442
]
Benedict commented on CASSANDRA-8683:
-
So I introduced the following code to the end of getPosition:
{code}
assert op == SSTableReader.Operator.EQ;
if (updateCacheAndStats)
bloomFilterTracker.addFalsePositive();
Tracing.trace(Partition index lookup complete (bloom filter false
positive) for sstable {}, descriptor.generation);
return null;
{code}
and this resulted in the following output (after enough runs):
{noformat}
[junit] Testcase:
testValidationMultipleSSTablePerLevel(org.apache.cassandra.db.compaction.LeveledCompactionStrategyTest):
Caused an ERROR
[junit] java.lang.AssertionError
[junit] java.util.concurrent.ExecutionException: java.lang.AssertionError
[junit] at java.util.concurrent.FutureTask.report(FutureTask.java:122)
[junit] at java.util.concurrent.FutureTask.get(FutureTask.java:188)
[junit] at
org.apache.cassandra.db.compaction.LeveledCompactionStrategyTest.testValidationMultipleSSTablePerLevel(LeveledCompactionStrategyTest.java:184)
[junit] Caused by: java.lang.AssertionError
[junit] at
org.apache.cassandra.io.sstable.format.big.BigTableReader.getPosition(BigTableReader.java:243)
[junit] at
org.apache.cassandra.io.sstable.format.SSTableReader.getPosition(SSTableReader.java:1355)
[junit] at
org.apache.cassandra.io.sstable.format.SSTableReader.getPositionsForRanges(SSTableReader.java:1282)
[junit] at
org.apache.cassandra.io.sstable.format.big.BigTableScanner.getScanner(BigTableScanner.java:67)
[junit] at
org.apache.cassandra.io.sstable.format.big.BigTableReader.getScanner(BigTableReader.java:101)
[junit] at
org.apache.cassandra.io.sstable.format.SSTableReader.getScanner(SSTableReader.java:1538)
[junit] at
org.apache.cassandra.db.compaction.LeveledCompactionStrategy$LeveledScanner.init(LeveledCompactionStrategy.java:318)
[junit] at
org.apache.cassandra.db.compaction.LeveledCompactionStrategy.getScanners(LeveledCompactionStrategy.java:245)
[junit] at
org.apache.cassandra.db.compaction.WrappingCompactionStrategy.getScanners(WrappingCompactionStrategy.java:357)
[junit] at
org.apache.cassandra.db.compaction.CompactionManager.doValidationCompaction(CompactionManager.java:999)
[junit] at
org.apache.cassandra.db.compaction.CompactionManager.access$600(CompactionManager.java:95)
[junit] at
org.apache.cassandra.db.compaction.CompactionManager$9.call(CompactionManager.java:591)
[junit] at java.util.concurrent.FutureTask.run(FutureTask.java:262)
[junit] at
java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1145)
[junit] at
java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:615)
[junit] at java.lang.Thread.run(Thread.java:745)
{noformat}
Which suggests that it is indeed that we're skipping over the entire contents
of our index page. However this may have nothing to do with the
getToken().maxKeyBound() business, since that would likely be deterministic
(although this is likely also a bug). It's possible that the memory being freed
is happening at the right timing interval to give us a bad binary search
result, though. This seems a little unlikely, but _is_ possible, so I suggest
we get 7705 finished and running so we can see nail down the early release of
memory, and then continue our investigations here.
Incremental repairs broken with early opening of compaction results
---
Key: CASSANDRA-8683
URL: https://issues.apache.org/jira/browse/CASSANDRA-8683
Project: Cassandra
Issue Type: Bug
Reporter: Marcus Eriksson
Assignee: Marcus Eriksson
Fix For: 2.1.3
Attachments: 0001-avoid-NPE-in-getPositionsForRanges.patch
Incremental repairs holds a set of the sstables it started the repair on (we
need to know which sstables were actually validated to be able to anticompact
them). This includes any tmplink files that existed when the compaction
started (if we wouldn't include those, we would miss data since we move the
start point of the existing non-tmplink files)
With CASSANDRA-6916 we swap out those instances with new ones
(SSTR.cloneWithNewStart / SSTW.openEarly), meaning that the underlying file
can get deleted even though we hold a reference.
This causes the unit test error:
http://cassci.datastax.com/job/trunk_utest/1330/testReport/junit/org.apache.cassandra.db.compaction/LeveledCompactionStrategyTest/testValidationMultipleSSTablePerLevel/
(note that it only fails on trunk though, in 2.1 we don't hold references to
the
[jira] [Commented] (CASSANDRA-8683) Incremental repairs broken with early opening of compaction results
[ https://issues.apache.org/jira/browse/CASSANDRA-8683?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14291873#comment-14291873 ] Benedict commented on CASSANDRA-8683: - I'm a little confused by the description: if it holds a reference, it shouldn't delete the file. Is it possible it's releasing its reference early somehow? Incremental repairs broken with early opening of compaction results --- Key: CASSANDRA-8683 URL: https://issues.apache.org/jira/browse/CASSANDRA-8683 Project: Cassandra Issue Type: Bug Reporter: Marcus Eriksson Assignee: Marcus Eriksson Fix For: 2.1.3 Incremental repairs holds a set of the sstables it started the repair on (we need to know which sstables were actually validated to be able to anticompact them). This includes any tmplink files that existed when the compaction started (if we wouldn't include those, we would miss data since we move the start point of the existing non-tmplink files) With CASSANDRA-6916 we swap out those instances with new ones (SSTR.cloneWithNewStart / SSTW.openEarly), meaning that the underlying file can get deleted even though we hold a reference. This causes the unit test error: http://cassci.datastax.com/job/trunk_utest/1330/testReport/junit/org.apache.cassandra.db.compaction/LeveledCompactionStrategyTest/testValidationMultipleSSTablePerLevel/ (note that it only fails on trunk though, in 2.1 we don't hold references to the repairing files for non-incremental repairs, but the bug should exist in 2.1 as well) -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (CASSANDRA-8683) Incremental repairs broken with early opening of compaction results
[
https://issues.apache.org/jira/browse/CASSANDRA-8683?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14291895#comment-14291895
]
Marcus Eriksson commented on CASSANDRA-8683:
from irc:
{noformat}
15:27 marcuse belliottsmith: we create a new sstable reader over the same
file, the reference counter is not transfered to the new file, so if i bump the
reference count on the file i hold a reference to, it will not affect the
latest one
15:27 marcuse does it make sense/
15:27 marcuse * transfered to the new SSTR instance
15:28 belliottsmith but you should have taken a reference against the one
you're using, no?
15:28 marcuse belliottsmith: no, we don't want to hold references during the
whole duration of the repair (hours)
15:28 belliottsmith each instance is distinct, but so long as you take a
reference against the one you're using, it shouldn't be a problem...
15:28 marcuse so, if a file is gone, we simply remove it from the set and
dont anticompact it
15:29 belliottsmith ah
15:29 belliottsmith hmm. that is tricky, since we need to repeatedly refetch
the set of files we want to use, since we could have new data replacing it as
well, surely?
15:30 belliottsmith i don't really see a way around holding a reference for
the lifetime of the repair. but if we stream oldest files first we can drop
references to them as we go, no?
15:30 marcuse no, for incremental repairs we repair best-effort, if a file
is gone, it means it has been compacted away and will get repaired next time
around
15:31 belliottsmith so where/when do we grab our reference to the sstable
then?
15:31 belliottsmith because in that case it sounds like we should simply be
failing to grab our reference count
15:31 marcuse
https://github.com/apache/cassandra/blob/trunk/src/java/org/apache/cassandra/service/ActiveRepairService.java#L409
15:32 marcuse and yes, we probably should fail, but we don't
15:32 belliottsmith that code looks a bit odd. why do we check for data
component first? shouldn't an attempt to acquire the ref be sufficient?
(clearly it isn't here though, since it's not removing it from the set)
15:33 marcuse yes, it should, but i don't think we decrease ref count when
we replace an instance?
15:34 marcuse ie, when we open early and have a new instance, we should
decrease the reference on the old file
15:34 marcuse *old instance\
15:34 belliottsmith we have to
15:34 belliottsmith otherwise it would never get deleted
15:34 marcuse but its the same underlying file right? it will get deleted by
the new instance
15:34 belliottsmith if we don't that's definitely a bug, but i'm pretty sure
we do
15:34 belliottsmith but lifetimes don't expire in order necessarily
15:35 belliottsmith so the older files ref count to zero, then remove
themselves from the linked-list of replacements
15:35 belliottsmith if there's nobody left, they delete the file
15:36 belliottsmith this all is in dire need of cleaning up in 8568
{noformat}
Incremental repairs broken with early opening of compaction results
---
Key: CASSANDRA-8683
URL: https://issues.apache.org/jira/browse/CASSANDRA-8683
Project: Cassandra
Issue Type: Bug
Reporter: Marcus Eriksson
Assignee: Marcus Eriksson
Fix For: 2.1.3
Incremental repairs holds a set of the sstables it started the repair on (we
need to know which sstables were actually validated to be able to anticompact
them). This includes any tmplink files that existed when the compaction
started (if we wouldn't include those, we would miss data since we move the
start point of the existing non-tmplink files)
With CASSANDRA-6916 we swap out those instances with new ones
(SSTR.cloneWithNewStart / SSTW.openEarly), meaning that the underlying file
can get deleted even though we hold a reference.
This causes the unit test error:
http://cassci.datastax.com/job/trunk_utest/1330/testReport/junit/org.apache.cassandra.db.compaction/LeveledCompactionStrategyTest/testValidationMultipleSSTablePerLevel/
(note that it only fails on trunk though, in 2.1 we don't hold references to
the repairing files for non-incremental repairs, but the bug should exist in
2.1 as well)
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (CASSANDRA-8683) Incremental repairs broken with early opening of compaction results
[ https://issues.apache.org/jira/browse/CASSANDRA-8683?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14291991#comment-14291991 ] Alan Boudreault commented on CASSANDRA-8683: related to CASSANDRA-8366 ? Incremental repairs broken with early opening of compaction results --- Key: CASSANDRA-8683 URL: https://issues.apache.org/jira/browse/CASSANDRA-8683 Project: Cassandra Issue Type: Bug Reporter: Marcus Eriksson Assignee: Marcus Eriksson Fix For: 2.1.3 Incremental repairs holds a set of the sstables it started the repair on (we need to know which sstables were actually validated to be able to anticompact them). This includes any tmplink files that existed when the compaction started (if we wouldn't include those, we would miss data since we move the start point of the existing non-tmplink files) With CASSANDRA-6916 we swap out those instances with new ones (SSTR.cloneWithNewStart / SSTW.openEarly), meaning that the underlying file can get deleted even though we hold a reference. This causes the unit test error: http://cassci.datastax.com/job/trunk_utest/1330/testReport/junit/org.apache.cassandra.db.compaction/LeveledCompactionStrategyTest/testValidationMultipleSSTablePerLevel/ (note that it only fails on trunk though, in 2.1 we don't hold references to the repairing files for non-incremental repairs, but the bug should exist in 2.1 as well) -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (CASSANDRA-8683) Incremental repairs broken with early opening of compaction results
[ https://issues.apache.org/jira/browse/CASSANDRA-8683?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14291865#comment-14291865 ] Marcus Eriksson commented on CASSANDRA-8683: cc [~benedict] Incremental repairs broken with early opening of compaction results --- Key: CASSANDRA-8683 URL: https://issues.apache.org/jira/browse/CASSANDRA-8683 Project: Cassandra Issue Type: Bug Reporter: Marcus Eriksson Assignee: Marcus Eriksson Fix For: 2.1.3 Incremental repairs holds a set of the sstables it started the repair on (we need to know which sstables were actually validated to be able to anticompact them). This includes any tmplink files that existed when the compaction started (if we wouldn't include those, we would miss data since we move the start point of the existing non-tmplink files) With CASSANDRA-6916 we swap out those instances with new ones (SSTR.cloneWithNewStart / SSTW.openEarly), meaning that the underlying file can get deleted even though we hold a reference. This causes the unit test error: http://cassci.datastax.com/job/trunk_utest/1330/testReport/junit/org.apache.cassandra.db.compaction/LeveledCompactionStrategyTest/testValidationMultipleSSTablePerLevel/ (note that it only fails on trunk though, in 2.1 we don't hold references to the repairing files for non-incremental repairs, but the bug should exist in 2.1 as well) -- This message was sent by Atlassian JIRA (v6.3.4#6332)
