Repository: cassandra Updated Branches: refs/heads/trunk 5ba79cbf2 -> 1f9d74c91
Fix RangeTombstone combining in RowAndTombstonMergeIterator patch by blerer; reviewed by slebresne for CASSANDRA-9759 Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/1f9d74c9 Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/1f9d74c9 Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/1f9d74c9 Branch: refs/heads/trunk Commit: 1f9d74c91318e974f70b2456969d4c75ce2dbfb9 Parents: 5ba79cb Author: blerer <[email protected]> Authored: Wed Jul 8 16:28:23 2015 +0200 Committer: Sylvain Lebresne <[email protected]> Committed: Mon Jul 13 15:45:21 2015 +0200 ---------------------------------------------------------------------- CHANGES.txt | 2 +- .../db/rows/RowAndTombstoneMergeIterator.java | 6 +- .../rows/RowAndTombstoneMergeIteratorTest.java | 415 +++++++++++++++++++ 3 files changed, 420 insertions(+), 3 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cassandra/blob/1f9d74c9/CHANGES.txt ---------------------------------------------------------------------- diff --git a/CHANGES.txt b/CHANGES.txt index 17d8f8a..590084f 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,7 +1,7 @@ 3.0 * Change CREATE/ALTER TABLE syntax for compression (CASSANDRA-8384) * Cleanup crc and adler code for java 8 (CASSANDRA-9650) - * Storage engine refactor (CASSANDRA-8099, 9743, 9746) + * Storage engine refactor (CASSANDRA-8099, 9743, 9746, 9759) * Update Guava to 18.0 (CASSANDRA-9653) * Bloom filter false positive ratio is not honoured (CASSANDRA-8413) * New option for cassandra-stress to leave a ratio of columns null (CASSANDRA-9522) http://git-wip-us.apache.org/repos/asf/cassandra/blob/1f9d74c9/src/java/org/apache/cassandra/db/rows/RowAndTombstoneMergeIterator.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/db/rows/RowAndTombstoneMergeIterator.java b/src/java/org/apache/cassandra/db/rows/RowAndTombstoneMergeIterator.java index f923329..3d204d3 100644 --- a/src/java/org/apache/cassandra/db/rows/RowAndTombstoneMergeIterator.java +++ b/src/java/org/apache/cassandra/db/rows/RowAndTombstoneMergeIterator.java @@ -86,7 +86,8 @@ public class RowAndTombstoneMergeIterator extends UnmodifiableIterator<Unfiltere RangeTombstone rt = nextTombstone; nextTombstone = tombstoneIter.hasNext() ? tombstoneIter.next() : null; // An end and a start makes a boundary if they sort similarly - if (nextTombstone != null && RangeTombstone.Bound.Kind.compare(rt.deletedSlice().close(reversed).kind(), nextTombstone.deletedSlice().open(reversed).kind()) == 0) + if (nextTombstone != null + && comparator.compare(rt.deletedSlice().close(reversed), nextTombstone.deletedSlice().open(reversed)) == 0) { next = RangeTombstoneBoundaryMarker.makeBoundary(reversed, rt.deletedSlice().close(reversed), @@ -112,7 +113,8 @@ public class RowAndTombstoneMergeIterator extends UnmodifiableIterator<Unfiltere { RangeTombstone rt = nextTombstone; nextTombstone = tombstoneIter.hasNext() ? tombstoneIter.next() : null; - if (nextTombstone != null && RangeTombstone.Bound.Kind.compare(rt.deletedSlice().close(reversed).kind(), nextTombstone.deletedSlice().open(reversed).kind()) == 0) + if (nextTombstone != null + && comparator.compare(rt.deletedSlice().close(reversed), nextTombstone.deletedSlice().open(reversed)) == 0) { next = RangeTombstoneBoundaryMarker.makeBoundary(reversed, rt.deletedSlice().close(reversed), http://git-wip-us.apache.org/repos/asf/cassandra/blob/1f9d74c9/test/unit/org/apache/cassandra/db/rows/RowAndTombstoneMergeIteratorTest.java ---------------------------------------------------------------------- diff --git a/test/unit/org/apache/cassandra/db/rows/RowAndTombstoneMergeIteratorTest.java b/test/unit/org/apache/cassandra/db/rows/RowAndTombstoneMergeIteratorTest.java new file mode 100644 index 0000000..88a6f7e --- /dev/null +++ b/test/unit/org/apache/cassandra/db/rows/RowAndTombstoneMergeIteratorTest.java @@ -0,0 +1,415 @@ +package org.apache.cassandra.db.rows; + +import java.nio.ByteBuffer; +import java.util.Collections; +import java.util.Iterator; + +import org.junit.Before; +import org.junit.BeforeClass; +import org.junit.Test; + +import org.apache.cassandra.db.Slice.Bound; +import org.apache.cassandra.db.ClusteringPrefix; +import org.apache.cassandra.utils.ByteBufferUtil; +import org.apache.cassandra.db.SimpleDeletionTime; +import org.apache.cassandra.db.Slice; +import org.apache.cassandra.db.RangeTombstone; +import org.apache.cassandra.db.RangeTombstoneList; +import org.apache.cassandra.db.LivenessInfo; +import org.apache.cassandra.db.SimpleLivenessInfo; +import org.apache.cassandra.db.marshal.AbstractType; +import org.apache.cassandra.config.ColumnDefinition; +import org.apache.cassandra.cql3.ColumnIdentifier; +import org.apache.cassandra.db.partitions.PartitionUpdate; +import org.apache.cassandra.db.marshal.Int32Type; +import org.apache.cassandra.SchemaLoader; +import org.apache.cassandra.Util; +import org.apache.cassandra.config.CFMetaData; +import org.apache.cassandra.db.ColumnFamilyStore; +import org.apache.cassandra.db.DecoratedKey; +import org.apache.cassandra.db.Keyspace; +import org.apache.cassandra.db.marshal.AsciiType; +import org.apache.cassandra.exceptions.ConfigurationException; +import org.apache.cassandra.schema.KeyspaceParams; +import org.apache.cassandra.utils.FBUtilities; +import static org.junit.Assert.*; + +public class RowAndTombstoneMergeIteratorTest +{ + private static final String KEYSPACE1 = "RowTest"; + private static final String CF_STANDARD1 = "Standard1"; + + private int nowInSeconds; + private DecoratedKey dk; + private ColumnFamilyStore cfs; + private CFMetaData cfm; + private ColumnDefinition defA; + + @BeforeClass + public static void defineSchema() throws ConfigurationException + { + CFMetaData cfMetadata = CFMetaData.Builder.create(KEYSPACE1, CF_STANDARD1) + .addPartitionKey("key", AsciiType.instance) + .addClusteringColumn("col1", Int32Type.instance) + .addRegularColumn("a", Int32Type.instance) + .build(); + SchemaLoader.prepareServer(); + SchemaLoader.createKeyspace(KEYSPACE1, + KeyspaceParams.simple(1), + cfMetadata); + + } + + @Before + public void setup() + { + nowInSeconds = FBUtilities.nowInSeconds(); + dk = Util.dk("key0"); + cfs = Keyspace.open(KEYSPACE1).getColumnFamilyStore(CF_STANDARD1); + cfm = cfs.metadata; + defA = cfm.getColumnDefinition(new ColumnIdentifier("a", true)); + } + + @Test + public void testWithNoRangeTombstones() + { + Iterator<Row> rowIterator = createRowIterator(); + + RowAndTombstoneMergeIterator iterator = new RowAndTombstoneMergeIterator(cfm.comparator, false) + .setTo(rowIterator, Collections.emptyIterator()); + + assertTrue(iterator.hasNext()); + assertRow(iterator.next(), 0); + + assertTrue(iterator.hasNext()); + assertRow(iterator.next(), 1); + + assertTrue(iterator.hasNext()); + assertRow(iterator.next(), 2); + + assertTrue(iterator.hasNext()); + assertRow(iterator.next(), 3); + + assertTrue(iterator.hasNext()); + assertRow(iterator.next(), 4); + + assertFalse(iterator.hasNext()); + } + + @Test + public void testWithOnlyRangeTombstones() + { + int delTime = nowInSeconds + 1; + long timestamp = delTime * 1000; + + Iterator<RangeTombstone> rangeTombstoneIterator = createRangeTombstoneIterator(rt(1, false, 3, false, timestamp, delTime), + atLeast(4, timestamp, delTime)); + RowAndTombstoneMergeIterator iterator = new RowAndTombstoneMergeIterator(cfm.comparator, false) + .setTo(Collections.emptyIterator(), rangeTombstoneIterator); + + assertTrue(iterator.hasNext()); + assertRtMarker(iterator.next(), ClusteringPrefix.Kind.EXCL_START_BOUND, 1); + + assertTrue(iterator.hasNext()); + assertRtMarker(iterator.next(), ClusteringPrefix.Kind.EXCL_END_BOUND, 3); + + assertTrue(iterator.hasNext()); + assertRtMarker(iterator.next(), ClusteringPrefix.Kind.INCL_START_BOUND, 4); + + assertTrue(iterator.hasNext()); + assertRtMarker(iterator.next(), Bound.TOP); + + assertFalse(iterator.hasNext()); + } + + @Test + public void testWithAtMostRangeTombstone() + { + Iterator<Row> rowIterator = createRowIterator(); + + int delTime = nowInSeconds + 1; + long timestamp = delTime * 1000; + + Iterator<RangeTombstone> rangeTombstoneIterator = createRangeTombstoneIterator(atMost(0, timestamp, delTime)); + + RowAndTombstoneMergeIterator iterator = new RowAndTombstoneMergeIterator(cfm.comparator, false) + .setTo(rowIterator, rangeTombstoneIterator); + + assertTrue(iterator.hasNext()); + assertRtMarker(iterator.next(), Bound.BOTTOM); + + assertTrue(iterator.hasNext()); + assertRow(iterator.next(), 0); + + assertTrue(iterator.hasNext()); + assertRtMarker(iterator.next(), ClusteringPrefix.Kind.INCL_END_BOUND, 0); + + assertTrue(iterator.hasNext()); + assertRow(iterator.next(), 1); + + assertTrue(iterator.hasNext()); + assertRow(iterator.next(), 2); + + assertTrue(iterator.hasNext()); + assertRow(iterator.next(), 3); + + assertTrue(iterator.hasNext()); + assertRow(iterator.next(), 4); + + assertFalse(iterator.hasNext()); + } + + @Test + public void testWithGreaterThanRangeTombstone() + { + Iterator<Row> rowIterator = createRowIterator(); + + int delTime = nowInSeconds + 1; + long timestamp = delTime * 1000; + + Iterator<RangeTombstone> rangeTombstoneIterator = createRangeTombstoneIterator(greaterThan(2, timestamp, delTime)); + + RowAndTombstoneMergeIterator iterator = new RowAndTombstoneMergeIterator(cfm.comparator, false) + .setTo(rowIterator, rangeTombstoneIterator); + + assertTrue(iterator.hasNext()); + assertRow(iterator.next(), 0); + + assertTrue(iterator.hasNext()); + assertRow(iterator.next(), 1); + + assertTrue(iterator.hasNext()); + assertRow(iterator.next(), 2); + + assertTrue(iterator.hasNext()); + assertRtMarker(iterator.next(), ClusteringPrefix.Kind.EXCL_START_BOUND, 2); + + assertTrue(iterator.hasNext()); + assertRow(iterator.next(), 3); + + assertTrue(iterator.hasNext()); + assertRow(iterator.next(), 4); + + assertTrue(iterator.hasNext()); + assertRtMarker(iterator.next(), Bound.TOP); + + assertFalse(iterator.hasNext()); + } + + @Test + public void testWithAtMostAndGreaterThanRangeTombstone() + { + Iterator<Row> rowIterator = createRowIterator(); + + int delTime = nowInSeconds + 1; + long timestamp = delTime * 1000; + + Iterator<RangeTombstone> rangeTombstoneIterator = createRangeTombstoneIterator(atMost(0, timestamp, delTime), + greaterThan(2, timestamp, delTime)); + + RowAndTombstoneMergeIterator iterator = new RowAndTombstoneMergeIterator(cfm.comparator, false) + .setTo(rowIterator, rangeTombstoneIterator); + + assertTrue(iterator.hasNext()); + assertRtMarker(iterator.next(), Bound.BOTTOM); + + assertTrue(iterator.hasNext()); + assertRow(iterator.next(), 0); + + assertTrue(iterator.hasNext()); + assertRtMarker(iterator.next(), ClusteringPrefix.Kind.INCL_END_BOUND, 0); + + assertTrue(iterator.hasNext()); + assertRow(iterator.next(), 1); + + assertTrue(iterator.hasNext()); + assertRow(iterator.next(), 2); + + assertTrue(iterator.hasNext()); + assertRtMarker(iterator.next(), ClusteringPrefix.Kind.EXCL_START_BOUND, 2); + + assertTrue(iterator.hasNext()); + assertRow(iterator.next(), 3); + + assertTrue(iterator.hasNext()); + assertRow(iterator.next(), 4); + + assertTrue(iterator.hasNext()); + assertRtMarker(iterator.next(), Bound.TOP); + + assertFalse(iterator.hasNext()); + } + + private void assertRtMarker(Unfiltered unfiltered, ClusteringPrefix.Kind kind, int col1) { + assertEquals(Unfiltered.Kind.RANGE_TOMBSTONE_MARKER, unfiltered.kind()); + assertEquals(kind, unfiltered.clustering().kind()); + assertEquals(bb(col1), unfiltered.clustering().get(0)); + } + + @Test + public void testWithIncludingEndExcludingStartMarker() + { + Iterator<Row> rowIterator = createRowIterator(); + + int delTime = nowInSeconds + 1; + long timestamp = delTime * 1000; + + Iterator<RangeTombstone> rangeTombstoneIterator = createRangeTombstoneIterator(atMost(2, timestamp, delTime), + greaterThan(2, timestamp, delTime)); + + RowAndTombstoneMergeIterator iterator = new RowAndTombstoneMergeIterator(cfm.comparator, false) + .setTo(rowIterator, rangeTombstoneIterator); + + assertTrue(iterator.hasNext()); + assertRtMarker(iterator.next(), Bound.BOTTOM); + + assertTrue(iterator.hasNext()); + assertRow(iterator.next(), 0); + + assertTrue(iterator.hasNext()); + assertRow(iterator.next(), 1); + + assertTrue(iterator.hasNext()); + assertRow(iterator.next(), 2); + + assertTrue(iterator.hasNext()); + assertRtMarker(iterator.next(), ClusteringPrefix.Kind.INCL_END_EXCL_START_BOUNDARY, 2); + + assertTrue(iterator.hasNext()); + assertRow(iterator.next(), 3); + + assertTrue(iterator.hasNext()); + assertRow(iterator.next(), 4); + + assertTrue(iterator.hasNext()); + assertRtMarker(iterator.next(), Bound.TOP); + + assertFalse(iterator.hasNext()); + } + + @Test + public void testWithExcludingEndIncludingStartMarker() + { + Iterator<Row> rowIterator = createRowIterator(); + + int delTime = nowInSeconds + 1; + long timestamp = delTime * 1000; + + Iterator<RangeTombstone> rangeTombstoneIterator = createRangeTombstoneIterator(lessThan(2, timestamp, delTime), + atLeast(2, timestamp, delTime)); + + RowAndTombstoneMergeIterator iterator = new RowAndTombstoneMergeIterator(cfm.comparator, false) + .setTo(rowIterator, rangeTombstoneIterator); + + assertTrue(iterator.hasNext()); + assertRtMarker(iterator.next(), Bound.BOTTOM); + + assertTrue(iterator.hasNext()); + assertRow(iterator.next(), 0); + + assertTrue(iterator.hasNext()); + assertRow(iterator.next(), 1); + + assertTrue(iterator.hasNext()); + assertRtMarker(iterator.next(), ClusteringPrefix.Kind.EXCL_END_INCL_START_BOUNDARY, 2); + + assertTrue(iterator.hasNext()); + assertRow(iterator.next(), 2); + + assertTrue(iterator.hasNext()); + assertRow(iterator.next(), 3); + + assertTrue(iterator.hasNext()); + assertRow(iterator.next(), 4); + + assertTrue(iterator.hasNext()); + assertRtMarker(iterator.next(), Bound.TOP); + + assertFalse(iterator.hasNext()); + } + + private void assertRtMarker(Unfiltered unfiltered, Bound bound) + { + assertEquals(Unfiltered.Kind.RANGE_TOMBSTONE_MARKER, unfiltered.kind()); + assertEquals(bound, unfiltered.clustering()); + } + + private void assertRow(Unfiltered unfiltered, int col1) + { + assertEquals(Unfiltered.Kind.ROW, unfiltered.kind()); + assertEquals(cfm.comparator.make(col1), unfiltered.clustering()); + } + + private Iterator<RangeTombstone> createRangeTombstoneIterator(RangeTombstone... tombstones) + { + RangeTombstoneList list = new RangeTombstoneList(cfm.comparator, 10); + + for (RangeTombstone tombstone : tombstones) + list.add(tombstone); + + return list.iterator(Slice.ALL, false); + } + + private Iterator<Row> createRowIterator() + { + PartitionUpdate update = new PartitionUpdate(cfm, dk, cfm.partitionColumns(), 1); + for (int i = 0; i < 5; i++) + addRow(update, i, i); + + return update.iterator(); + } + + private void addRow(PartitionUpdate update, int col1, int a) + { + Rows.writeClustering(update.metadata().comparator.make(col1), update.writer()); + writeSimpleCellValue(update.writer(), cfm, defA, a, 0, nowInSeconds); + update.writer().endOfRow(); + } + + private void writeSimpleCellValue(Row.Writer writer, + CFMetaData cfm, + ColumnDefinition columnDefinition, + int value, + long timestamp, + int nowInSeconds) + { + writer.writeCell(columnDefinition, + false, + ((AbstractType) columnDefinition.cellValueType()).decompose(value), + SimpleLivenessInfo.forUpdate(timestamp, LivenessInfo.NO_TTL, nowInSeconds, cfm), + null); + } + + private static RangeTombstone atLeast(int start, long tstamp, int delTime) + { + return new RangeTombstone(Slice.make(Slice.Bound.inclusiveStartOf(bb(start)), Slice.Bound.TOP), new SimpleDeletionTime(tstamp, delTime)); + } + + private static RangeTombstone atMost(int end, long tstamp, int delTime) + { + return new RangeTombstone(Slice.make(Slice.Bound.BOTTOM, Slice.Bound.inclusiveEndOf(bb(end))), new SimpleDeletionTime(tstamp, delTime)); + } + + private static RangeTombstone lessThan(int end, long tstamp, int delTime) + { + return new RangeTombstone(Slice.make(Slice.Bound.BOTTOM, Slice.Bound.exclusiveEndOf(bb(end))), new SimpleDeletionTime(tstamp, delTime)); + } + + private static RangeTombstone greaterThan(int start, long tstamp, int delTime) + { + return new RangeTombstone(Slice.make(Slice.Bound.exclusiveStartOf(bb(start)), Slice.Bound.TOP), new SimpleDeletionTime(tstamp, delTime)); + } + + private static RangeTombstone rt(int start, boolean startInclusive, int end, boolean endInclusive, long tstamp, int delTime) + { + Slice.Bound startBound = startInclusive ? Slice.Bound.inclusiveStartOf(bb(start)) : Slice.Bound.exclusiveStartOf(bb(start)); + Slice.Bound endBound = endInclusive ? Slice.Bound.inclusiveEndOf(bb(end)) : Slice.Bound.exclusiveEndOf(bb(end)); + + return new RangeTombstone(Slice.make(startBound, endBound), new SimpleDeletionTime(tstamp, delTime)); + } + + private static ByteBuffer bb(int i) + { + return ByteBufferUtil.bytes(i); + } +}
