Repository: cassandra
Updated Branches:
  refs/heads/cassandra-3.0 9be437064 -> 06c55f779
  refs/heads/cassandra-3.11 2725d5035 -> 5d5588204
  refs/heads/trunk 047bcd7ad -> d44059bfc


Static deletions are corrupted in 3.0 -> 2.{1,2} messages (again)

The prior fix was incorrect; it turns out serialization to 2.{1,2} nodes was 
broken as well,
not just deserialization from 2.{1,2} nodes.

patch by Benedict; reviewed by Aleksey and Sylvain for CASSANDRA-14568


Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo
Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/68dbeb34
Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/68dbeb34
Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/68dbeb34

Branch: refs/heads/cassandra-3.0
Commit: 68dbeb34c9404ee3cd7db00cc112e27c9a4b1f6f
Parents: 9be4370
Author: Benedict Elliott Smith <bened...@apple.com>
Authored: Wed Aug 15 18:38:07 2018 +0100
Committer: Benedict Elliott Smith <bened...@apple.com>
Committed: Fri Sep 14 11:16:50 2018 +0100

----------------------------------------------------------------------
 CHANGES.txt                                     |  1 +
 .../org/apache/cassandra/db/LegacyLayout.java   | 34 ++++++++++++++------
 .../apache/cassandra/db/LegacyLayoutTest.java   |  1 -
 3 files changed, 26 insertions(+), 10 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cassandra/blob/68dbeb34/CHANGES.txt
----------------------------------------------------------------------
diff --git a/CHANGES.txt b/CHANGES.txt
index 12c16b7..037e2a8 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,4 +1,5 @@
 3.0.18
+ * Fix corrupted static collection deletions in 3.0 <-> 2.{1,2} messages 
(CASSANDRA-14568)
  * Handle failures in parallelAllSSTableOperation 
(cleanup/upgradesstables/etc) (CASSANDRA-14657)
  * Improve TokenMetaData cache populating performance avoid long locking 
(CASSANDRA-14660)
  * Fix static column order for SELECT * wildcard queries (CASSANDRA-14638)

http://git-wip-us.apache.org/repos/asf/cassandra/blob/68dbeb34/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 e0f66e3..2115c7d 100644
--- a/src/java/org/apache/cassandra/db/LegacyLayout.java
+++ b/src/java/org/apache/cassandra/db/LegacyLayout.java
@@ -25,7 +25,6 @@ import java.security.MessageDigest;
 import java.util.*;
 
 import org.apache.cassandra.cql3.SuperColumnCompatibility;
-import org.apache.cassandra.thrift.Column;
 import org.apache.cassandra.utils.AbstractIterator;
 import com.google.common.collect.Iterators;
 import com.google.common.collect.Lists;
@@ -46,6 +45,7 @@ import org.apache.cassandra.utils.*;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import static com.google.common.collect.Iterables.all;
 import static org.apache.cassandra.utils.ByteBufferUtil.bytes;
 
 /**
@@ -201,16 +201,19 @@ public abstract class LegacyLayout
         List<ByteBuffer> components = CompositeType.splitName(bound);
         byte eoc = CompositeType.lastEOC(bound);
 
+        // if the bound we have decoded is static, 2.2 format requires there 
to be N empty clusterings
+        assert !isStatic ||
+                (components.size() >= clusteringSize
+                        && all(components.subList(0, clusteringSize), 
ByteBufferUtil.EMPTY_BYTE_BUFFER::equals));
         // There can be  more components than the clustering size only in the 
case this is the bound of a collection
         // range tombstone. In which case, there is exactly one more 
component, and that component is the name of the
         // collection being selected/deleted.
-        assert components.size() <= clusteringSize || 
(!metadata.isCompactTable() && components.size() == clusteringSize + 1);
-
         ColumnDefinition collectionName = null;
-        if (components.size() > (isStatic ? 0 : clusteringSize))
+        if (components.size() > clusteringSize)
         {
+            assert clusteringSize + 1 == components.size() && 
!metadata.isCompactTable();
             // pop the collection name from the back of the list of clusterings
-            ByteBuffer collectionNameBytes = components.remove(isStatic ? 0 : 
clusteringSize);
+            ByteBuffer collectionNameBytes = components.remove(clusteringSize);
             collectionName = metadata.getColumnDefinition(collectionNameBytes);
         }
 
@@ -799,12 +802,18 @@ public abstract class LegacyLayout
             if (!delTime.isLive())
             {
                 Clustering clustering = row.clustering();
+                boolean isStatic = clustering == Clustering.STATIC_CLUSTERING;
+                assert isStatic == col.isStatic();
 
-                Slice.Bound startBound = 
Slice.Bound.inclusiveStartOf(clustering);
-                Slice.Bound endBound = Slice.Bound.inclusiveEndOf(clustering);
+                Slice.Bound startBound = isStatic
+                        ? LegacyDeletionInfo.staticBound(metadata, true)
+                        : Slice.Bound.inclusiveStartOf(clustering);
+                Slice.Bound endBound = isStatic
+                        ? LegacyDeletionInfo.staticBound(metadata, false)
+                        : Slice.Bound.inclusiveEndOf(clustering);
 
-                LegacyLayout.LegacyBound start = new 
LegacyLayout.LegacyBound(startBound, col.isStatic(), col);
-                LegacyLayout.LegacyBound end = new 
LegacyLayout.LegacyBound(endBound, col.isStatic(), col);
+                LegacyLayout.LegacyBound start = new 
LegacyLayout.LegacyBound(startBound, isStatic, col);
+                LegacyLayout.LegacyBound end = new 
LegacyLayout.LegacyBound(endBound, isStatic, col);
 
                 deletions.add(start, end, delTime.markedForDeleteAt(), 
delTime.localDeletionTime());
             }
@@ -1496,6 +1505,12 @@ public abstract class LegacyLayout
     {
         public boolean isCell();
 
+        // note that for static atoms, LegacyCell and LegacyRangeTombstone 
behave differently here:
+        //  - LegacyCell returns the modern Clustering.STATIC_CLUSTERING
+        //  - LegacyRangeTombstone returns the 2.2 bound (i.e. N empty 
ByteBuffer, where N is number of clusterings)
+        // in LegacyDeletionInfo.add(), we split any LRT with a static bound 
out into the inRowRangeTombstones collection
+        // these are merged with regular row cells, in the CellGrouper, and 
their clustering is obtained via start.bound.getAsClustering
+        // (also, it should be impossibly to issue raw static row deletions 
anyway)
         public ClusteringPrefix clustering();
         public boolean isStatic();
 
@@ -1689,6 +1704,7 @@ public abstract class LegacyLayout
             this.deletionTime = deletionTime;
         }
 
+        /** @see LegacyAtom#clustering for static inconsistencies explained */
         public ClusteringPrefix clustering()
         {
             return start.bound;

http://git-wip-us.apache.org/repos/asf/cassandra/blob/68dbeb34/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 44d8a70..ce818c0 100644
--- a/test/unit/org/apache/cassandra/db/LegacyLayoutTest.java
+++ b/test/unit/org/apache/cassandra/db/LegacyLayoutTest.java
@@ -187,7 +187,6 @@ public class LegacyLayoutTest
         }
     }
 
-
     @Test
     public void testStaticRangeTombstoneRoundTripUnexpectedDeletion() throws 
Throwable
     {


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org

Reply via email to