Extra range tombstone bound creates double rows in 3.0 Patch by Jeff Jirsa; Reviewed by Aleksey Yeschenko for CASSANDRA-14008
Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/4a2b516a Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/4a2b516a Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/4a2b516a Branch: refs/heads/trunk Commit: 4a2b516a3488ab04ee3338e74397b8c6d69e6d43 Parents: dd187d1 Author: Jeff Jirsa <jji...@apple.com> Authored: Sat Nov 11 09:10:22 2017 -0800 Committer: Jeff Jirsa <jji...@apple.com> Committed: Tue Dec 12 09:52:19 2017 -0800 ---------------------------------------------------------------------- CHANGES.txt | 1 + .../org/apache/cassandra/db/LegacyLayout.java | 103 ++++++++++++------- ...egacy_ka_repeated_rt-ka-1-CompressionInfo.db | Bin 0 -> 43 bytes ...Keyspace1-legacy_ka_repeated_rt-ka-1-Data.db | Bin 0 -> 422 bytes ...pace1-legacy_ka_repeated_rt-ka-1-Digest.sha1 | 1 + ...yspace1-legacy_ka_repeated_rt-ka-1-Filter.db | Bin 0 -> 176 bytes ...eyspace1-legacy_ka_repeated_rt-ka-1-Index.db | Bin 0 -> 570 bytes ...ce1-legacy_ka_repeated_rt-ka-1-Statistics.db | Bin 0 -> 4478 bytes ...space1-legacy_ka_repeated_rt-ka-1-Summary.db | Bin 0 -> 92 bytes ...Keyspace1-legacy_ka_repeated_rt-ka-1-TOC.txt | 8 ++ .../apache/cassandra/db/LegacyLayoutTest.java | 83 +++++++++++++++ 11 files changed, 159 insertions(+), 37 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cassandra/blob/4a2b516a/CHANGES.txt ---------------------------------------------------------------------- diff --git a/CHANGES.txt b/CHANGES.txt index 20ccc4b..1ca7902 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 3.0.16 + * Extra range tombstone bound creates double rows (CASSANDRA-14008) * Fix SStable ordering by max timestamp in SinglePartitionReadCommand (CASSANDRA-14010) * Accept role names containing forward-slash (CASSANDRA-14088) * Optimize CRC check chance probability calculations (CASSANDRA-14094) http://git-wip-us.apache.org/repos/asf/cassandra/blob/4a2b516a/src/java/org/apache/cassandra/db/LegacyLayout.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/db/LegacyLayout.java b/src/java/org/apache/cassandra/db/LegacyLayout.java index 3ba96a6..2117dd6 100644 --- a/src/java/org/apache/cassandra/db/LegacyLayout.java +++ b/src/java/org/apache/cassandra/db/LegacyLayout.java @@ -1273,56 +1273,85 @@ public abstract class LegacyLayout return true; } - public boolean addRangeTombstone(LegacyRangeTombstone tombstone) + private boolean addRangeTombstone(LegacyRangeTombstone tombstone) { if (tombstone.isRowDeletion(metadata)) + return addRowTombstone(tombstone); + else if (tombstone.isCollectionTombstone()) + return addCollectionTombstone(tombstone); + else + return addGenericRangeTombstone(tombstone); + } + + private boolean addRowTombstone(LegacyRangeTombstone tombstone) + { + if (clustering != null) { - if (clustering != null) + // If we're already in the row, there might be a chance that there were two range tombstones + // written, as 2.x storage format does not guarantee just one range tombstone, unlike 3.x. + // We have to make sure that clustering matches, which would mean that tombstone is for the + // same row. + if (rowDeletion != null && clustering.equals(tombstone.start.getAsClustering(metadata))) { - // If we're already in the row, there might be a chance that there were two range tombstones - // written, as 2.x storage format does not guarantee just one range tombstone, unlike 3.x. - // We have to make sure that clustering matches, which would mean that tombstone is for the - // same row. - if (rowDeletion != null && clustering.equals(tombstone.start.getAsClustering(metadata))) + // If the tombstone superceeds the previous delete, we discard the previous one + if (tombstone.deletionTime.supersedes(rowDeletion.deletionTime)) { - // If the tombstone superceeds the previous delete, we discard the previous one - if (tombstone.deletionTime.supersedes(rowDeletion.deletionTime)) - { - builder.addRowDeletion(Row.Deletion.regular(tombstone.deletionTime)); - rowDeletion = tombstone; - } - return true; + builder.addRowDeletion(Row.Deletion.regular(tombstone.deletionTime)); + rowDeletion = tombstone; } - - // If we're already within a row and there was no delete written before that one, it can't be the same one - return false; + return true; } + // If we're already within a row and there was no delete written before that one, it can't be the same one + return false; + } + + clustering = tombstone.start.getAsClustering(metadata); + builder.newRow(clustering); + builder.addRowDeletion(Row.Deletion.regular(tombstone.deletionTime)); + rowDeletion = tombstone; + + return true; + } + + private boolean addCollectionTombstone(LegacyRangeTombstone tombstone) + { + if (!helper.includes(tombstone.start.collectionName)) + return false; // see CASSANDRA-13109 + + if (clustering == null) + { clustering = tombstone.start.getAsClustering(metadata); builder.newRow(clustering); - builder.addRowDeletion(Row.Deletion.regular(tombstone.deletionTime)); - rowDeletion = tombstone; - return true; } - - if (tombstone.isCollectionTombstone() && helper.includes(tombstone.start.collectionName)) + else if (!clustering.equals(tombstone.start.getAsClustering(metadata))) { - if (clustering == null) - { - clustering = tombstone.start.getAsClustering(metadata); - builder.newRow(clustering); - } - else if (!clustering.equals(tombstone.start.getAsClustering(metadata))) - { - return false; - } - - builder.addComplexDeletion(tombstone.start.collectionName, tombstone.deletionTime); - if (rowDeletion == null || tombstone.deletionTime.supersedes(rowDeletion.deletionTime)) - collectionDeletion = tombstone; - return true; + return false; } - return false; + + builder.addComplexDeletion(tombstone.start.collectionName, tombstone.deletionTime); + if (rowDeletion == null || tombstone.deletionTime.supersedes(rowDeletion.deletionTime)) + collectionDeletion = tombstone; + + return true; + } + + private boolean addGenericRangeTombstone(LegacyRangeTombstone tombstone) + { + /* + * We can see a non-collection, non-row deletion in two scenarios: + * + * 1. Most commonly, the tombstone's start bound is bigger than current row's clustering, which means that + * the current row is over, and we should move on to the next row or RT; + * + * 2. Less commonly, the tombstone's start bound is smaller than current row's clustering, which means that + * we've crossed an index boundary and are seeing a non-closed RT from the previous block, repeated; + * we should ignore it and stay in the current row. + * + * In either case, clustering should be non-null, or we shouldn't have gotten to this method at all + * However, to be absolutely SURE we're in case two above, we check here. + */ + return clustering != null && metadata.comparator.compare(clustering, tombstone.start.bound.clustering()) > 0; } public Row getRow() http://git-wip-us.apache.org/repos/asf/cassandra/blob/4a2b516a/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_repeated_rt/Keyspace1-legacy_ka_repeated_rt-ka-1-CompressionInfo.db ---------------------------------------------------------------------- diff --git a/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_repeated_rt/Keyspace1-legacy_ka_repeated_rt-ka-1-CompressionInfo.db b/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_repeated_rt/Keyspace1-legacy_ka_repeated_rt-ka-1-CompressionInfo.db new file mode 100644 index 0000000..9a33154 Binary files /dev/null and b/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_repeated_rt/Keyspace1-legacy_ka_repeated_rt-ka-1-CompressionInfo.db differ http://git-wip-us.apache.org/repos/asf/cassandra/blob/4a2b516a/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_repeated_rt/Keyspace1-legacy_ka_repeated_rt-ka-1-Data.db ---------------------------------------------------------------------- diff --git a/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_repeated_rt/Keyspace1-legacy_ka_repeated_rt-ka-1-Data.db b/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_repeated_rt/Keyspace1-legacy_ka_repeated_rt-ka-1-Data.db new file mode 100644 index 0000000..80a7c46 Binary files /dev/null and b/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_repeated_rt/Keyspace1-legacy_ka_repeated_rt-ka-1-Data.db differ http://git-wip-us.apache.org/repos/asf/cassandra/blob/4a2b516a/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_repeated_rt/Keyspace1-legacy_ka_repeated_rt-ka-1-Digest.sha1 ---------------------------------------------------------------------- diff --git a/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_repeated_rt/Keyspace1-legacy_ka_repeated_rt-ka-1-Digest.sha1 b/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_repeated_rt/Keyspace1-legacy_ka_repeated_rt-ka-1-Digest.sha1 new file mode 100644 index 0000000..de07755 --- /dev/null +++ b/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_repeated_rt/Keyspace1-legacy_ka_repeated_rt-ka-1-Digest.sha1 @@ -0,0 +1 @@ +1973536272 \ No newline at end of file http://git-wip-us.apache.org/repos/asf/cassandra/blob/4a2b516a/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_repeated_rt/Keyspace1-legacy_ka_repeated_rt-ka-1-Filter.db ---------------------------------------------------------------------- diff --git a/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_repeated_rt/Keyspace1-legacy_ka_repeated_rt-ka-1-Filter.db b/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_repeated_rt/Keyspace1-legacy_ka_repeated_rt-ka-1-Filter.db new file mode 100644 index 0000000..dfcab1f Binary files /dev/null and b/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_repeated_rt/Keyspace1-legacy_ka_repeated_rt-ka-1-Filter.db differ http://git-wip-us.apache.org/repos/asf/cassandra/blob/4a2b516a/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_repeated_rt/Keyspace1-legacy_ka_repeated_rt-ka-1-Index.db ---------------------------------------------------------------------- diff --git a/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_repeated_rt/Keyspace1-legacy_ka_repeated_rt-ka-1-Index.db b/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_repeated_rt/Keyspace1-legacy_ka_repeated_rt-ka-1-Index.db new file mode 100644 index 0000000..9fefd10 Binary files /dev/null and b/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_repeated_rt/Keyspace1-legacy_ka_repeated_rt-ka-1-Index.db differ http://git-wip-us.apache.org/repos/asf/cassandra/blob/4a2b516a/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_repeated_rt/Keyspace1-legacy_ka_repeated_rt-ka-1-Statistics.db ---------------------------------------------------------------------- diff --git a/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_repeated_rt/Keyspace1-legacy_ka_repeated_rt-ka-1-Statistics.db b/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_repeated_rt/Keyspace1-legacy_ka_repeated_rt-ka-1-Statistics.db new file mode 100644 index 0000000..77c6233 Binary files /dev/null and b/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_repeated_rt/Keyspace1-legacy_ka_repeated_rt-ka-1-Statistics.db differ http://git-wip-us.apache.org/repos/asf/cassandra/blob/4a2b516a/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_repeated_rt/Keyspace1-legacy_ka_repeated_rt-ka-1-Summary.db ---------------------------------------------------------------------- diff --git a/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_repeated_rt/Keyspace1-legacy_ka_repeated_rt-ka-1-Summary.db b/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_repeated_rt/Keyspace1-legacy_ka_repeated_rt-ka-1-Summary.db new file mode 100644 index 0000000..0c15fd4 Binary files /dev/null and b/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_repeated_rt/Keyspace1-legacy_ka_repeated_rt-ka-1-Summary.db differ http://git-wip-us.apache.org/repos/asf/cassandra/blob/4a2b516a/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_repeated_rt/Keyspace1-legacy_ka_repeated_rt-ka-1-TOC.txt ---------------------------------------------------------------------- diff --git a/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_repeated_rt/Keyspace1-legacy_ka_repeated_rt-ka-1-TOC.txt b/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_repeated_rt/Keyspace1-legacy_ka_repeated_rt-ka-1-TOC.txt new file mode 100644 index 0000000..a78243a --- /dev/null +++ b/test/data/legacy-sstables/ka/legacy_tables/legacy_ka_repeated_rt/Keyspace1-legacy_ka_repeated_rt-ka-1-TOC.txt @@ -0,0 +1,8 @@ +Data.db +TOC.txt +Digest.sha1 +Index.db +CompressionInfo.db +Filter.db +Summary.db +Statistics.db http://git-wip-us.apache.org/repos/asf/cassandra/blob/4a2b516a/test/unit/org/apache/cassandra/db/LegacyLayoutTest.java ---------------------------------------------------------------------- diff --git a/test/unit/org/apache/cassandra/db/LegacyLayoutTest.java b/test/unit/org/apache/cassandra/db/LegacyLayoutTest.java index 09e9755..715d7c9 100644 --- a/test/unit/org/apache/cassandra/db/LegacyLayoutTest.java +++ b/test/unit/org/apache/cassandra/db/LegacyLayoutTest.java @@ -19,17 +19,26 @@ package org.apache.cassandra.db; import java.nio.ByteBuffer; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; import org.junit.Test; +import org.apache.cassandra.SchemaLoader; import org.apache.cassandra.config.CFMetaData; import org.apache.cassandra.config.ColumnDefinition; +import org.apache.cassandra.config.DatabaseDescriptor; import org.apache.cassandra.cql3.ColumnIdentifier; +import org.apache.cassandra.cql3.QueryProcessor; +import org.apache.cassandra.cql3.UntypedResultSet; import org.apache.cassandra.db.marshal.Int32Type; import org.apache.cassandra.db.marshal.SetType; import org.apache.cassandra.db.partitions.PartitionUpdate; import org.apache.cassandra.db.rows.BTreeRow; import org.apache.cassandra.db.rows.Row; +import org.apache.cassandra.dht.Murmur3Partitioner; +import org.apache.cassandra.schema.KeyspaceParams; import org.apache.cassandra.utils.ByteBufferUtil; import static org.junit.Assert.*; @@ -68,4 +77,78 @@ public class LegacyLayoutTest assertEquals("b", l.starts[1].collectionName.name.toString()); assertEquals("b", l.ends[1].collectionName.name.toString()); } + + /** + * Tests with valid sstables containing duplicate RT entries at index boundaries + * in 2.1 format, where DATA below is a > 1000 byte long string of letters, + * and the column index is set to 1kb + + [ + {"key": "1", + "cells": [["1:_","1:!",1513015245,"t",1513015263], + ["1:1:","",1513015467727335], + ["1:1:val1","DATA",1513015467727335], + ["1:_","1:!",1513015245,"t",1513015263], + ["1:1:val2","DATA",1513015467727335], + ["1:_","1:!",1513015245,"t",1513015263], + ["1:1:val3","DATA",1513015467727335], + ["1:_","1:!",1513015245,"t",1513015263], + ["1:2:","",1513015458470156], + ["1:2:val1","DATA",1513015458470156], + ["1:_","1:!",1513015245,"t",1513015263], + ["1:2:val2","DATA",1513015458470156], + ["1:_","1:!",1513015245,"t",1513015263], + ["1:2:val3","DATA",1513015458470156], + ["1:_","1:!",1513015245,"t",1513015263], + ["1:3:","",1513015450253602], + ["1:3:val1","DATA",1513015450253602], + ["1:_","1:!",1513015245,"t",1513015263], + ["1:3:val2","DATA",1513015450253602], + ["1:_","1:!",1513015245,"t",1513015263], + ["1:3:val3","DATA",1513015450253602]]} + ] + * + * See CASSANDRA-14008 for details. + */ + @Test + public void testRTBetweenColumns() throws Throwable + { + String KEYSPACE = "Keyspace1"; + DatabaseDescriptor.setPartitionerUnsafe(Murmur3Partitioner.instance); + + SchemaLoader.loadSchema(); + SchemaLoader.createKeyspace(KEYSPACE, KeyspaceParams.simple(1)); + QueryProcessor.executeInternal(String.format("CREATE TABLE \"%s\".legacy_ka_repeated_rt (k1 int, c1 int, c2 int, val1 text, val2 text, val3 text, primary key (k1, c1, c2))", KEYSPACE)); + + Keyspace keyspace = Keyspace.open(KEYSPACE); + ColumnFamilyStore cfs = keyspace.getColumnFamilyStore("legacy_ka_repeated_rt"); + + Path legacySSTableRoot = Paths.get("test/data/legacy-sstables/ka/legacy_tables/legacy_ka_repeated_rt/"); + + for (String filename : new String[]{ "Keyspace1-legacy_ka_repeated_rt-ka-1-CompressionInfo.db", + "Keyspace1-legacy_ka_repeated_rt-ka-1-Data.db", + "Keyspace1-legacy_ka_repeated_rt-ka-1-Digest.sha1", + "Keyspace1-legacy_ka_repeated_rt-ka-1-Filter.db", + "Keyspace1-legacy_ka_repeated_rt-ka-1-Index.db", + "Keyspace1-legacy_ka_repeated_rt-ka-1-Statistics.db", + "Keyspace1-legacy_ka_repeated_rt-ka-1-Summary.db", + "Keyspace1-legacy_ka_repeated_rt-ka-1-TOC.txt" }) + { + Files.copy(Paths.get(legacySSTableRoot.toString(), filename), cfs.getDirectories().getDirectoryForNewSSTables().toPath().resolve(filename)); + } + + cfs.loadNewSSTables(); + + UntypedResultSet rs = QueryProcessor.executeInternal(String.format("SELECT * FROM \"%s\".legacy_ka_repeated_rt WHERE k1=1", KEYSPACE)); + assertEquals(3, rs.size()); + UntypedResultSet rs2 = QueryProcessor.executeInternal(String.format("SELECT * FROM \"%s\".legacy_ka_repeated_rt WHERE k1=1 AND c1=1", KEYSPACE)); + assertEquals(3, rs2.size()); + for (int i = 1; i <= 3; i++) + { + UntypedResultSet rs3 = QueryProcessor.executeInternal(String.format("SELECT * FROM \"%s\".legacy_ka_repeated_rt WHERE k1=1 AND c1=1 AND c2=%s", KEYSPACE, i)); + assertEquals(1, rs3.size()); + + } + + } } \ No newline at end of file --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org