You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by jj...@apache.org on 2017/12/12 17:59:36 UTC
[3/6] cassandra git commit: Extra range tombstone bound creates
double rows in 3.0
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 <jj...@apple.com>
Authored: Sat Nov 11 09:10:22 2017 -0800
Committer: Jeff Jirsa <jj...@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-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org