cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jji...@apache.org
Subject [2/6] cassandra git commit: Extra range tombstone bound creates double rows in 3.0
Date Tue, 12 Dec 2017 17:59:35 GMT
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/cassandra-3.11
Commit: 4a2b516a3488ab04ee3338e74397b8c6d69e6d43
Parents: dd187d1
Author: Jeff Jirsa <jjirsa@apple.com>
Authored: Sat Nov 11 09:10:22 2017 -0800
Committer: Jeff Jirsa <jjirsa@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


Mime
View raw message