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/trunk 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