You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by be...@apache.org on 2018/09/14 10:26:01 UTC
[2/9] cassandra git commit: Static deletions are corrupted in 3.0 ->
2.{1, 2} messages (again)
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.11
Commit: 68dbeb34c9404ee3cd7db00cc112e27c9a4b1f6f
Parents: 9be4370
Author: Benedict Elliott Smith <be...@apple.com>
Authored: Wed Aug 15 18:38:07 2018 +0100
Committer: Benedict Elliott Smith <be...@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-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org