Reverse order reads can return incomplete results Patch by Blake Eggleston; Reviewed by Sam Tunnicliffe for CASSANDRA-14803
Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/ab0e30e7 Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/ab0e30e7 Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/ab0e30e7 Branch: refs/heads/cassandra-3.11 Commit: ab0e30e75904e4d637f07b2ec64334073eb061ec Parents: 30d2835 Author: Blake Eggleston <[email protected]> Authored: Wed Oct 3 15:53:04 2018 -0700 Committer: Blake Eggleston <[email protected]> Committed: Thu Oct 4 12:46:21 2018 -0700 ---------------------------------------------------------------------- CHANGES.txt | 1 + .../columniterator/SSTableReversedIterator.java | 40 +++++++++++++------ ...bles-legacy_ka_14803-ka-1-CompressionInfo.db | Bin 0 -> 43 bytes .../legacy_tables-legacy_ka_14803-ka-1-Data.db | Bin 0 -> 16555 bytes ...gacy_tables-legacy_ka_14803-ka-1-Digest.sha1 | 1 + ...legacy_tables-legacy_ka_14803-ka-1-Filter.db | Bin 0 -> 16 bytes .../legacy_tables-legacy_ka_14803-ka-1-Index.db | Bin 0 -> 206 bytes ...cy_tables-legacy_ka_14803-ka-1-Statistics.db | Bin 0 -> 4450 bytes ...egacy_tables-legacy_ka_14803-ka-1-Summary.db | Bin 0 -> 92 bytes .../legacy_tables-legacy_ka_14803-ka-1-TOC.txt | 8 ++++ .../cassandra/io/sstable/LegacySSTableTest.java | 21 +++++++++- 11 files changed, 58 insertions(+), 13 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cassandra/blob/ab0e30e7/CHANGES.txt ---------------------------------------------------------------------- diff --git a/CHANGES.txt b/CHANGES.txt index 70b2996..3c6d3b5 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 3.0.18 + * Reverse order reads can return incomplete results (CASSANDRA-14803) * Avoid calling iter.next() in a loop when notifying indexers about range tombstones (CASSANDRA-14794) * Fix purging semi-expired RT boundaries in reversed iterators (CASSANDRA-14672) * DESC order reads can fail to return the last Unfiltered in the partition (CASSANDRA-14766) http://git-wip-us.apache.org/repos/asf/cassandra/blob/ab0e30e7/src/java/org/apache/cassandra/db/columniterator/SSTableReversedIterator.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/db/columniterator/SSTableReversedIterator.java b/src/java/org/apache/cassandra/db/columniterator/SSTableReversedIterator.java index d5b46a4..f8c0e77 100644 --- a/src/java/org/apache/cassandra/db/columniterator/SSTableReversedIterator.java +++ b/src/java/org/apache/cassandra/db/columniterator/SSTableReversedIterator.java @@ -20,6 +20,8 @@ package org.apache.cassandra.db.columniterator; import java.io.IOException; import java.util.*; +import com.google.common.base.Preconditions; + import org.apache.cassandra.config.CFMetaData; import org.apache.cassandra.db.*; import org.apache.cassandra.db.filter.ColumnFilter; @@ -314,18 +316,32 @@ public class SSTableReversedIterator extends AbstractSSTableIterator if (super.hasNextInternal()) return true; - // We have nothing more for our current block, move the next one (so the one before on disk). - int nextBlockIdx = indexState.currentBlockIdx() - 1; - if (nextBlockIdx < 0 || nextBlockIdx < lastBlockIdx) - return false; - - // The slice start can be in - indexState.setToBlock(nextBlockIdx); - readCurrentBlock(true, nextBlockIdx != lastBlockIdx); - // since that new block is within the bounds we've computed in setToSlice(), we know there will - // always be something matching the slice unless we're on the lastBlockIdx (in which case there - // may or may not be results, but if there isn't, we're done for the slice). - return iterator.hasNext(); + while (true) + { + // We have nothing more for our current block, move the next one (so the one before on disk). + int nextBlockIdx = indexState.currentBlockIdx() - 1; + if (nextBlockIdx < 0 || nextBlockIdx < lastBlockIdx) + return false; + + // The slice start can be in + indexState.setToBlock(nextBlockIdx); + readCurrentBlock(true, nextBlockIdx != lastBlockIdx); + + // for pre-3.0 storage formats, index blocks that only contain a single row and that row crosses + // index boundaries, the iterator will be empty even though we haven't read everything we're intending + // to read. In that case, we want to read the next index block. This shouldn't be possible in 3.0+ + // formats (see next comment) + if (!iterator.hasNext() && nextBlockIdx > lastBlockIdx) + { + Preconditions.checkState(!sstable.descriptor.version.storeRows()); + continue; + } + + // for 3.0+ storage formats, since that new block is within the bounds we've computed in setToSlice(), + // we know there will always be something matching the slice unless we're on the lastBlockIdx (in which + // case there may or may not be results, but if there isn't, we're done for the slice). + return iterator.hasNext(); + } } /** http://git-wip-us.apache.org/repos/asf/cassandra/blob/ab0e30e7/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_14803/legacy_tables-legacy_ka_14803-ka-1-CompressionInfo.db ---------------------------------------------------------------------- diff --git a/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_14803/legacy_tables-legacy_ka_14803-ka-1-CompressionInfo.db b/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_14803/legacy_tables-legacy_ka_14803-ka-1-CompressionInfo.db new file mode 100644 index 0000000..bb15937 Binary files /dev/null and b/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_14803/legacy_tables-legacy_ka_14803-ka-1-CompressionInfo.db differ http://git-wip-us.apache.org/repos/asf/cassandra/blob/ab0e30e7/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_14803/legacy_tables-legacy_ka_14803-ka-1-Data.db ---------------------------------------------------------------------- diff --git a/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_14803/legacy_tables-legacy_ka_14803-ka-1-Data.db b/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_14803/legacy_tables-legacy_ka_14803-ka-1-Data.db new file mode 100644 index 0000000..9f946ab Binary files /dev/null and b/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_14803/legacy_tables-legacy_ka_14803-ka-1-Data.db differ http://git-wip-us.apache.org/repos/asf/cassandra/blob/ab0e30e7/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_14803/legacy_tables-legacy_ka_14803-ka-1-Digest.sha1 ---------------------------------------------------------------------- diff --git a/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_14803/legacy_tables-legacy_ka_14803-ka-1-Digest.sha1 b/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_14803/legacy_tables-legacy_ka_14803-ka-1-Digest.sha1 new file mode 100644 index 0000000..ec58891 --- /dev/null +++ b/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_14803/legacy_tables-legacy_ka_14803-ka-1-Digest.sha1 @@ -0,0 +1 @@ +2454867855 \ No newline at end of file http://git-wip-us.apache.org/repos/asf/cassandra/blob/ab0e30e7/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_14803/legacy_tables-legacy_ka_14803-ka-1-Filter.db ---------------------------------------------------------------------- diff --git a/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_14803/legacy_tables-legacy_ka_14803-ka-1-Filter.db b/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_14803/legacy_tables-legacy_ka_14803-ka-1-Filter.db new file mode 100644 index 0000000..606783d Binary files /dev/null and b/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_14803/legacy_tables-legacy_ka_14803-ka-1-Filter.db differ http://git-wip-us.apache.org/repos/asf/cassandra/blob/ab0e30e7/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_14803/legacy_tables-legacy_ka_14803-ka-1-Index.db ---------------------------------------------------------------------- diff --git a/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_14803/legacy_tables-legacy_ka_14803-ka-1-Index.db b/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_14803/legacy_tables-legacy_ka_14803-ka-1-Index.db new file mode 100644 index 0000000..bcf40a1 Binary files /dev/null and b/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_14803/legacy_tables-legacy_ka_14803-ka-1-Index.db differ http://git-wip-us.apache.org/repos/asf/cassandra/blob/ab0e30e7/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_14803/legacy_tables-legacy_ka_14803-ka-1-Statistics.db ---------------------------------------------------------------------- diff --git a/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_14803/legacy_tables-legacy_ka_14803-ka-1-Statistics.db b/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_14803/legacy_tables-legacy_ka_14803-ka-1-Statistics.db new file mode 100644 index 0000000..d30baa5 Binary files /dev/null and b/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_14803/legacy_tables-legacy_ka_14803-ka-1-Statistics.db differ http://git-wip-us.apache.org/repos/asf/cassandra/blob/ab0e30e7/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_14803/legacy_tables-legacy_ka_14803-ka-1-Summary.db ---------------------------------------------------------------------- diff --git a/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_14803/legacy_tables-legacy_ka_14803-ka-1-Summary.db b/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_14803/legacy_tables-legacy_ka_14803-ka-1-Summary.db new file mode 100644 index 0000000..a4d9a6e Binary files /dev/null and b/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_14803/legacy_tables-legacy_ka_14803-ka-1-Summary.db differ http://git-wip-us.apache.org/repos/asf/cassandra/blob/ab0e30e7/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_14803/legacy_tables-legacy_ka_14803-ka-1-TOC.txt ---------------------------------------------------------------------- diff --git a/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_14803/legacy_tables-legacy_ka_14803-ka-1-TOC.txt b/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_14803/legacy_tables-legacy_ka_14803-ka-1-TOC.txt new file mode 100644 index 0000000..141f12c --- /dev/null +++ b/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_14803/legacy_tables-legacy_ka_14803-ka-1-TOC.txt @@ -0,0 +1,8 @@ +Summary.db +Data.db +Index.db +Digest.sha1 +CompressionInfo.db +TOC.txt +Filter.db +Statistics.db http://git-wip-us.apache.org/repos/asf/cassandra/blob/ab0e30e7/test/unit/org/apache/cassandra/io/sstable/LegacySSTableTest.java ---------------------------------------------------------------------- diff --git a/test/unit/org/apache/cassandra/io/sstable/LegacySSTableTest.java b/test/unit/org/apache/cassandra/io/sstable/LegacySSTableTest.java index f10114b..f6467e1 100644 --- a/test/unit/org/apache/cassandra/io/sstable/LegacySSTableTest.java +++ b/test/unit/org/apache/cassandra/io/sstable/LegacySSTableTest.java @@ -27,7 +27,6 @@ import java.util.Random; import org.junit.After; import org.junit.Assert; -import org.junit.Before; import org.junit.BeforeClass; import org.junit.Ignore; import org.junit.Test; @@ -213,6 +212,26 @@ public class LegacySSTableTest } @Test + public void test14803() throws Exception + { + /* + * During upgrades from 2.1 to 3.0, reading from old sstables in reverse order could return early if the sstable + * reverse iterator encounters an indexed block that only covers a single row, and that row starts in the next + * indexed block. + */ + + QueryProcessor.executeInternal("CREATE TABLE legacy_tables.legacy_ka_14803 (k int, c int, v1 blob, v2 blob, PRIMARY KEY (k, c));"); + loadLegacyTable("legacy_%s_14803%s", "ka", ""); + + UntypedResultSet forward = QueryProcessor.executeOnceInternal(String.format("SELECT * FROM legacy_tables.legacy_ka_14803 WHERE k=100")); + UntypedResultSet reverse = QueryProcessor.executeOnceInternal(String.format("SELECT * FROM legacy_tables.legacy_ka_14803 WHERE k=100 ORDER BY c DESC")); + + logger.info("{} - {}", forward.size(), reverse.size()); + Assert.assertFalse(forward.isEmpty()); + Assert.assertEquals(forward.size(), reverse.size()); + } + + @Test public void testVerifyOldSSTables() throws Exception { for (String legacyVersion : legacyVersions) --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
