This is an automated email from the ASF dual-hosted git repository. slebresne pushed a commit to branch cassandra-3.11 in repository https://gitbox.apache.org/repos/asf/cassandra.git
commit ebfd05254f84000f71fa018650632d24d3761f07 Merge: 3cda9d7 c8a2834 Author: Sylvain Lebresne <lebre...@gmail.com> AuthorDate: Wed May 27 17:12:44 2020 +0200 Merge commit 'c8a2834606d683ba9945e9cc11bdb4207ce269d1' into cassandra-3.11 CHANGES.txt | 1 + src/java/org/apache/cassandra/db/LegacyLayout.java | 105 +++++++++++++---- .../cassandra/db/UnfilteredDeserializer.java | 129 ++++++++++++++++----- .../upgrade/MixedModeRangeTombstoneTest.java | 73 ++++++++++++ .../org/apache/cassandra/db/LegacyLayoutTest.java | 102 +++++++++++++--- 5 files changed, 340 insertions(+), 70 deletions(-) diff --cc CHANGES.txt index 11515c4,46b3f56..a809016 --- a/CHANGES.txt +++ b/CHANGES.txt @@@ -1,7 -1,5 +1,8 @@@ -3.0.21 +3.11.7 + * Fix CQL formatting of read command restrictions for slow query log (CASSANDRA-15503) + * Allow sstableloader to use SSL on the native port (CASSANDRA-14904) +Merged from 3.0: + * Fix duplicated row on 2.x upgrades when multi-rows range tombstones interact with collection ones (CASSANDRA-15805) * Rely on snapshotted session infos on StreamResultFuture.maybeComplete to avoid race conditions (CASSANDRA-15667) * EmptyType doesn't override writeValue so could attempt to write bytes when expected not to (CASSANDRA-15790) * Fix index queries on partition key columns when some partitions contains only static data (CASSANDRA-13666) diff --cc src/java/org/apache/cassandra/db/LegacyLayout.java index 4ec0c30,8492de5..b28c72a --- a/src/java/org/apache/cassandra/db/LegacyLayout.java +++ b/src/java/org/apache/cassandra/db/LegacyLayout.java @@@ -1891,9 -1934,9 +1936,9 @@@ public abstract class LegacyLayou if ((start.collectionName == null) != (stop.collectionName == null)) { if (start.collectionName == null) - stop = new LegacyBound(stop.bound, stop.isStatic, null); - stop = new LegacyBound(Slice.Bound.inclusiveEndOf(stop.bound.values), stop.isStatic, null); ++ stop = new LegacyBound(ClusteringBound.inclusiveEndOf(stop.bound.values), stop.isStatic, null); else - start = new LegacyBound(start.bound, start.isStatic, null); - start = new LegacyBound(Slice.Bound.inclusiveStartOf(start.bound.values), start.isStatic, null); ++ start = new LegacyBound(ClusteringBound.inclusiveStartOf(start.bound.values), start.isStatic, null); } else if (!Objects.equals(start.collectionName, stop.collectionName)) { @@@ -1920,11 -1963,21 +1965,21 @@@ return new LegacyRangeTombstone(newStart, stop, deletionTime); } - public LegacyRangeTombstone withNewStart(Slice.Bound newStart) ++ public LegacyRangeTombstone withNewStart(ClusteringBound newStart) + { + return withNewStart(new LegacyBound(newStart, start.isStatic, null)); + } + public LegacyRangeTombstone withNewEnd(LegacyBound newStop) { return new LegacyRangeTombstone(start, newStop, deletionTime); } - public LegacyRangeTombstone withNewEnd(Slice.Bound newEnd) ++ public LegacyRangeTombstone withNewEnd(ClusteringBound newEnd) + { + return withNewEnd(new LegacyBound(newEnd, stop.isStatic, null)); + } + public boolean isCell() { return false; diff --cc src/java/org/apache/cassandra/db/UnfilteredDeserializer.java index cdcde2e,2d270bc..262b333 --- a/src/java/org/apache/cassandra/db/UnfilteredDeserializer.java +++ b/src/java/org/apache/cassandra/db/UnfilteredDeserializer.java @@@ -480,19 -480,9 +480,10 @@@ public abstract class UnfilteredDeseria this.helper = helper; this.grouper = new LegacyLayout.CellGrouper(metadata, helper); this.tombstoneTracker = new TombstoneTracker(partitionDeletion); - this.atoms = new AtomIterator(atomReader); - } - - private boolean isRow(LegacyLayout.LegacyAtom atom) - { - if (atom.isCell()) - return true; - - LegacyLayout.LegacyRangeTombstone tombstone = atom.asRangeTombstone(); - return tombstone.isCollectionTombstone() || tombstone.isRowDeletion(metadata); + this.atoms = new AtomIterator(atomReader, metadata); } + public boolean hasNext() { // Note that we loop on next == null because TombstoneTracker.openNew() could return null below or the atom might be shadowed. @@@ -540,13 -530,57 +531,57 @@@ ? LegacyLayout.CellGrouper.staticGrouper(metadata, helper) : this.grouper; grouper.reset(); + // We know the first atom is not shadowed and is a "row" atom, so can be added blindly. grouper.addAtom(first); - // As long as atoms are part of the same row, consume them. Note that the call to addAtom() uses - // atoms.peek() so that the atom is only consumed (by next) if it's part of the row (addAtom returns true) - while (atoms.hasNext() && grouper.addAtom(atoms.peek())) + + // We're less sure about the next atoms. In particular, CellGrouper want to make sure we only pass it + // "row" atoms (it's the only type it knows how to handle) so we should handle anything else. + while (atoms.hasNext()) { - atoms.next(); + // Peek, but don't consume the next atom just yet + LegacyLayout.LegacyAtom atom = atoms.peek(); + // First, that atom may be shadowed in which case we can simply ignore it. Note that this handles + // the case of repeated RT start marker after we've crossed an index boundary, which could well + // appear in the middle of a row (CASSANDRA-14008). + if (!tombstoneTracker.hasClosingMarkerBefore(atom) && tombstoneTracker.isShadowed(atom)) + { + atoms.next(); // consume the atom since we only peeked it so far + continue; + } + + // Second, we should only pass "row" atoms to the cell grouper + if (atom.isRowAtom(metadata)) + { + if (!grouper.addAtom(atom)) + break; // done with the row; don't consume the atom + atoms.next(); // the grouper "accepted" the atom, consume it since we only peeked above + } + else + { + LegacyLayout.LegacyRangeTombstone rt = (LegacyLayout.LegacyRangeTombstone) atom; + // This means we have a non-row range tombstone. Unfortunately, that does not guarantee the + // current row is finished (though it may), because due to the logic within LegacyRangeTombstone + // constructor, we can get an out-of-order RT that includes on the current row (even if it is + // already started) and extends past it. + + // So first, evacuate the easy case of the range tombstone simply starting after the current + // row, in which case we're done with the current row (but don't consume the new RT yet so it + // gets handled as any other non-row RT). + if (grouper.startsAfterCurrentRow(rt)) + break; + + // Otherwise, we "split" the RT in 2: the part covering the current row, which is now an + // inRowAtom and can be passed to the grouper, and the part after that, which we push back into + // the iterator for later processing. + Clustering currentRow = grouper.currentRowClustering(); + atoms.next(); // consume since we had only just peeked it so far and we're using it - atoms.pushOutOfOrder(rt.withNewStart(Slice.Bound.exclusiveStartOf(currentRow))); ++ atoms.pushOutOfOrder(rt.withNewStart(ClusteringBound.exclusiveStartOf(currentRow))); + // Note: in theory the withNewStart is a no-op here, but not taking any risk - grouper.addAtom(rt.withNewStart(Slice.Bound.inclusiveStartOf(currentRow)) - .withNewEnd(Slice.Bound.inclusiveEndOf(currentRow))); ++ grouper.addAtom(rt.withNewStart(ClusteringBound.inclusiveStartOf(currentRow)) ++ .withNewEnd(ClusteringBound.inclusiveEndOf(currentRow))); + } } + return grouper.getRow(); } diff --cc test/unit/org/apache/cassandra/db/LegacyLayoutTest.java index 1bc3af6,f0d2a02..65565e1 --- a/test/unit/org/apache/cassandra/db/LegacyLayoutTest.java +++ b/test/unit/org/apache/cassandra/db/LegacyLayoutTest.java @@@ -24,7 -24,10 +24,11 @@@ import java.nio.file.Files import java.nio.file.Path; import java.nio.file.Paths; +import org.junit.AfterClass; + import org.apache.cassandra.db.LegacyLayout.CellGrouper; + import org.apache.cassandra.db.LegacyLayout.LegacyBound; + import org.apache.cassandra.db.LegacyLayout.LegacyCell; + import org.apache.cassandra.db.LegacyLayout.LegacyRangeTombstone; import org.apache.cassandra.db.filter.ColumnFilter; import org.apache.cassandra.db.marshal.MapType; import org.apache.cassandra.db.marshal.UTF8Type; @@@ -397,26 -386,89 +398,89 @@@ public class LegacyLayoutTes SerializationHelper helper = new SerializationHelper(cfm, MessagingService.VERSION_22, SerializationHelper.Flag.LOCAL, ColumnFilter.all(cfm)); LegacyLayout.CellGrouper cg = new LegacyLayout.CellGrouper(cfm, helper); - ClusteringBound startBound = ClusteringBound.create(ClusteringPrefix.Kind.INCL_START_BOUND, new ByteBuffer[] {ByteBufferUtil.bytes(2)}); - ClusteringBound endBound = ClusteringBound.create(ClusteringPrefix.Kind.EXCL_END_BOUND, new ByteBuffer[] {ByteBufferUtil.bytes(2)}); - LegacyLayout.LegacyBound start = new LegacyLayout.LegacyBound(startBound, false, cfm.getColumnDefinition(ByteBufferUtil.bytes("v"))); - LegacyLayout.LegacyBound end = new LegacyLayout.LegacyBound(endBound, false, cfm.getColumnDefinition(ByteBufferUtil.bytes("v"))); - Slice.Bound startBound = Slice.Bound.create(ClusteringPrefix.Kind.INCL_START_BOUND, new ByteBuffer[] {bytes(2)}); - Slice.Bound endBound = Slice.Bound.create(ClusteringPrefix.Kind.EXCL_END_BOUND, new ByteBuffer[] {bytes(2)}); ++ ClusteringBound startBound = ClusteringBound.create(ClusteringPrefix.Kind.INCL_START_BOUND, new ByteBuffer[] {bytes(2)}); ++ ClusteringBound endBound = ClusteringBound.create(ClusteringPrefix.Kind.EXCL_END_BOUND, new ByteBuffer[] {bytes(2)}); + LegacyLayout.LegacyBound start = new LegacyLayout.LegacyBound(startBound, false, cfm.getColumnDefinition(bytes("v"))); + LegacyLayout.LegacyBound end = new LegacyLayout.LegacyBound(endBound, false, cfm.getColumnDefinition(bytes("v"))); LegacyLayout.LegacyRangeTombstone lrt = new LegacyLayout.LegacyRangeTombstone(start, end, new DeletionTime(2, 1588598040)); assertTrue(cg.addAtom(lrt)); // add a real cell LegacyLayout.LegacyCell cell = new LegacyLayout.LegacyCell(LegacyLayout.LegacyCell.Kind.REGULAR, - new LegacyLayout.LegacyCellName(Clustering.make(ByteBufferUtil.bytes(2)), - cfm.getColumnDefinition(ByteBufferUtil.bytes("v")), - ByteBufferUtil.bytes("g")), - ByteBufferUtil.bytes("v"), 3, Integer.MAX_VALUE, 0); - new LegacyLayout.LegacyCellName(new Clustering(bytes(2)), ++ new LegacyLayout.LegacyCellName(Clustering.make(bytes(2)), + cfm.getColumnDefinition(bytes("v")), + bytes("g")), + bytes("v"), 3, Integer.MAX_VALUE, 0); assertTrue(cg.addAtom(cell)); // add legacy range tombstone where collection name is null for the end bound (this gets translated to a row tombstone) - startBound = ClusteringBound.create(ClusteringPrefix.Kind.EXCL_START_BOUND, new ByteBuffer[] {ByteBufferUtil.bytes(2)}); - endBound = ClusteringBound.create(ClusteringPrefix.Kind.EXCL_END_BOUND, new ByteBuffer[] {ByteBufferUtil.bytes(2)}); - start = new LegacyLayout.LegacyBound(startBound, false, cfm.getColumnDefinition(ByteBufferUtil.bytes("v"))); - startBound = Slice.Bound.create(ClusteringPrefix.Kind.EXCL_START_BOUND, new ByteBuffer[] {bytes(2)}); - endBound = Slice.Bound.create(ClusteringPrefix.Kind.EXCL_END_BOUND, new ByteBuffer[] {bytes(2)}); ++ startBound = ClusteringBound.create(ClusteringPrefix.Kind.EXCL_START_BOUND, new ByteBuffer[] {bytes(2)}); ++ endBound = ClusteringBound.create(ClusteringPrefix.Kind.EXCL_END_BOUND, new ByteBuffer[] {bytes(2)}); + start = new LegacyLayout.LegacyBound(startBound, false, cfm.getColumnDefinition(bytes("v"))); end = new LegacyLayout.LegacyBound(endBound, false, null); assertTrue(cg.addAtom(new LegacyLayout.LegacyRangeTombstone(start, end, new DeletionTime(1, 1588598040)))); } + + private static LegacyCell cell(Clustering clustering, ColumnDefinition column, ByteBuffer value, long timestamp) + { + return new LegacyCell(LegacyCell.Kind.REGULAR, + new LegacyLayout.LegacyCellName(clustering, column, null), + value, + timestamp, + Cell.NO_DELETION_TIME, + Cell.NO_TTL); + } + + /** + * This tests that when {@link CellGrouper} gets a collection tombstone for + * a non-fetched collection, then that tombstone does not incorrectly stop the grouping of the current row, as + * was done before CASSANDRA-15805. + * + * <p>Please note that this rely on a query only _fetching_ some of the table columns, which in practice only + * happens for thrift queries, and thrift queries shouldn't mess up with CQL tables and collection tombstones, + * so this test is not of the utmost importance. Nonetheless, the pre-CASSANDRA-15805 behavior was incorrect and + * this ensure it is fixed. + */ + @Test + public void testCellGrouperOnNonFecthedCollectionTombstone() + { + // CREATE TABLE %s (pk int, ck int, a text, b set<text>, c text, PRIMARY KEY (pk, ck)) + CFMetaData cfm = CFMetaData.Builder.create("ks", "table") + .addPartitionKey("pk", Int32Type.instance) + .addClusteringColumn("ck", Int32Type.instance) + .addRegularColumn("a", UTF8Type.instance) + .addRegularColumn("b", SetType.getInstance(UTF8Type.instance, true)) + .addRegularColumn("c", UTF8Type.instance) + .build(); + + // Creates a filter that _only_ fetches a and c, but not b. + ColumnFilter filter = ColumnFilter.selectionBuilder() + .add(cfm.getColumnDefinition(bytes("a"))) + .add(cfm.getColumnDefinition(bytes("c"))) + .build(); + SerializationHelper helper = new SerializationHelper(cfm, + MessagingService.VERSION_22, + SerializationHelper.Flag.LOCAL, + filter); + CellGrouper grouper = new CellGrouper(cfm, helper); - Clustering clustering = new Clustering(bytes(1)); ++ Clustering clustering = new BufferClustering(bytes(1)); + + // We add a cell for a, then a collection tombstone for b, and then a cell for c (for the same clustering). + // All those additions should return 'true' as all belong to the same row. + LegacyCell ca = cell(clustering, cfm.getColumnDefinition(bytes("a")), bytes("v1"), 1); + assertTrue(grouper.addAtom(ca)); + - Slice.Bound startBound = Slice.Bound.inclusiveStartOf(bytes(1)); - Slice.Bound endBound = Slice.Bound.inclusiveEndOf(bytes(1)); ++ ClusteringBound startBound = ClusteringBound.inclusiveStartOf(bytes(1)); ++ ClusteringBound endBound = ClusteringBound.inclusiveEndOf(bytes(1)); + ColumnDefinition bDef = cfm.getColumnDefinition(bytes("b")); + assert bDef != null; + LegacyBound start = new LegacyBound(startBound, false, bDef); + LegacyBound end = new LegacyBound(endBound, false, bDef); + LegacyRangeTombstone rtb = new LegacyRangeTombstone(start, end, new DeletionTime(1, 1588598040)); + assertTrue(rtb.isCollectionTombstone()); // Ensure we're testing what we think + assertTrue(grouper.addAtom(rtb)); + + LegacyCell cc = cell(clustering, cfm.getColumnDefinition(bytes("c")), bytes("v2"), 1); + assertTrue(grouper.addAtom(cc)); + } } --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org