Repository: cassandra Updated Branches: refs/heads/3.0 [created] 2d6fd7824 refs/heads/cassandra-3.11 5efaaf91c -> 863ad11c8 refs/heads/trunk 94aa57e27 -> a1cb8e5ab
Make reading of range tombstones more reliable Patch by Alex Petrov; reviewed by Benjamin Lerer for CASSANDRA-12811 Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/2d6fd782 Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/2d6fd782 Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/2d6fd782 Branch: refs/heads/cassandra-3.11 Commit: 2d6fd782465395d54d8958e2da8a5c8744a81942 Parents: 833c993 Author: Alex Petrov <oleksandr.pet...@gmail.com> Authored: Fri Apr 7 12:09:32 2017 +0200 Committer: Alex Petrov <oleksandr.pet...@gmail.com> Committed: Fri Apr 7 12:10:46 2017 +0200 ---------------------------------------------------------------------- CHANGES.txt | 1 + .../db/SinglePartitionReadCommand.java | 11 +-- .../db/filter/ClusteringIndexNamesFilter.java | 6 +- .../db/partitions/AbstractBTreePartition.java | 5 -- .../cassandra/utils/IndexedSearchIterator.java | 5 ++ .../apache/cassandra/utils/SearchIterator.java | 2 - .../cql3/validation/operations/DeleteTest.java | 82 +++++++++++++++++++- .../partition/PartitionImplementationTest.java | 2 +- 8 files changed, 92 insertions(+), 22 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cassandra/blob/2d6fd782/CHANGES.txt ---------------------------------------------------------------------- diff --git a/CHANGES.txt b/CHANGES.txt index 33d5028..440ccd8 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 3.0.13 + * Make reading of range tombstones more reliable (CASSANDRA-12811) * Fix startup problems due to schema tables not completely flushed (CASSANDRA-12213) * Fix view builder bug that can filter out data on restart (CASSANDRA-13405) * Fix 2i page size calculation when there are no regular columns (CASSANDRA-13400) http://git-wip-us.apache.org/repos/asf/cassandra/blob/2d6fd782/src/java/org/apache/cassandra/db/SinglePartitionReadCommand.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/db/SinglePartitionReadCommand.java b/src/java/org/apache/cassandra/db/SinglePartitionReadCommand.java index 5f8df1b..99abd10 100644 --- a/src/java/org/apache/cassandra/db/SinglePartitionReadCommand.java +++ b/src/java/org/apache/cassandra/db/SinglePartitionReadCommand.java @@ -736,13 +736,13 @@ public class SinglePartitionReadCommand extends ReadCommand // We need to get the partition deletion and include it if it's live. In any case though, we're done with that sstable. sstable.incrementReadCount(); - try (UnfilteredRowIterator iter = sstable.iterator(partitionKey(), columnFilter(), filter.isReversed(), isForThrift())) + try (UnfilteredRowIterator iter = filter.filter(sstable.iterator(partitionKey(), columnFilter(), filter.isReversed(), isForThrift()))) { + sstablesIterated++; if (!iter.partitionLevelDeletion().isLive()) - { - sstablesIterated++; result = add(UnfilteredRowIterators.noRowsIterator(iter.metadata(), iter.partitionKey(), Rows.EMPTY_STATIC_ROW, iter.partitionLevelDeletion(), filter.isReversed()), result, filter, sstable.isRepaired()); - } + else + result = add(iter, result, filter, sstable.isRepaired()); } continue; } @@ -835,9 +835,6 @@ public class SinglePartitionReadCommand extends ReadCommand NavigableSet<Clustering> toRemove = null; for (Clustering clustering : clusterings) { - if (!searchIter.hasNext()) - break; - Row row = searchIter.next(clustering); if (row == null || !canRemoveRow(row, columns.regulars, sstableTimestamp)) continue; http://git-wip-us.apache.org/repos/asf/cassandra/blob/2d6fd782/src/java/org/apache/cassandra/db/filter/ClusteringIndexNamesFilter.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/db/filter/ClusteringIndexNamesFilter.java b/src/java/org/apache/cassandra/db/filter/ClusteringIndexNamesFilter.java index a81a7a6..7769f2e 100644 --- a/src/java/org/apache/cassandra/db/filter/ClusteringIndexNamesFilter.java +++ b/src/java/org/apache/cassandra/db/filter/ClusteringIndexNamesFilter.java @@ -176,7 +176,9 @@ public class ClusteringIndexNamesFilter extends AbstractClusteringIndexFilter public UnfilteredRowIterator getUnfilteredRowIterator(final ColumnFilter columnFilter, final Partition partition) { + final Iterator<Clustering> clusteringIter = clusteringsInQueryOrder.iterator(); final SearchIterator<Clustering, Row> searcher = partition.searchIterator(columnFilter, reversed); + return new AbstractUnfilteredRowIterator(partition.metadata(), partition.partitionKey(), partition.partitionLevelDeletion(), @@ -185,11 +187,9 @@ public class ClusteringIndexNamesFilter extends AbstractClusteringIndexFilter reversed, partition.stats()) { - private final Iterator<Clustering> clusteringIter = clusteringsInQueryOrder.iterator(); - protected Unfiltered computeNext() { - while (clusteringIter.hasNext() && searcher.hasNext()) + while (clusteringIter.hasNext()) { Row row = searcher.next(clusteringIter.next()); if (row != null) http://git-wip-us.apache.org/repos/asf/cassandra/blob/2d6fd782/src/java/org/apache/cassandra/db/partitions/AbstractBTreePartition.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/db/partitions/AbstractBTreePartition.java b/src/java/org/apache/cassandra/db/partitions/AbstractBTreePartition.java index c63acc2..2aa622e 100644 --- a/src/java/org/apache/cassandra/db/partitions/AbstractBTreePartition.java +++ b/src/java/org/apache/cassandra/db/partitions/AbstractBTreePartition.java @@ -139,11 +139,6 @@ public abstract class AbstractBTreePartition implements Partition, Iterable<Row> private final SearchIterator<Clustering, Row> rawIter = new BTreeSearchIterator<>(current.tree, metadata.comparator, desc(reversed)); private final DeletionTime partitionDeletion = current.deletionInfo.getPartitionDeletion(); - public boolean hasNext() - { - return rawIter.hasNext(); - } - public Row next(Clustering clustering) { if (clustering == Clustering.STATIC_CLUSTERING) http://git-wip-us.apache.org/repos/asf/cassandra/blob/2d6fd782/src/java/org/apache/cassandra/utils/IndexedSearchIterator.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/utils/IndexedSearchIterator.java b/src/java/org/apache/cassandra/utils/IndexedSearchIterator.java index a156629..597e5bb 100644 --- a/src/java/org/apache/cassandra/utils/IndexedSearchIterator.java +++ b/src/java/org/apache/cassandra/utils/IndexedSearchIterator.java @@ -20,6 +20,11 @@ package org.apache.cassandra.utils; public interface IndexedSearchIterator<K, V> extends SearchIterator<K, V> { /** + * @return true if iterator has any elements left, false otherwise + */ + public boolean hasNext(); + + /** * @return the value just recently returned by next() * @throws java.util.NoSuchElementException if next() returned null */ http://git-wip-us.apache.org/repos/asf/cassandra/blob/2d6fd782/src/java/org/apache/cassandra/utils/SearchIterator.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/utils/SearchIterator.java b/src/java/org/apache/cassandra/utils/SearchIterator.java index 5309f4a..908053b 100644 --- a/src/java/org/apache/cassandra/utils/SearchIterator.java +++ b/src/java/org/apache/cassandra/utils/SearchIterator.java @@ -19,8 +19,6 @@ package org.apache.cassandra.utils; public interface SearchIterator<K, V> { - public boolean hasNext(); - /** * Searches "forwards" (in direction of travel) in the iterator for the required key; * if this or any key greater has already been returned by the iterator, the method may http://git-wip-us.apache.org/repos/asf/cassandra/blob/2d6fd782/test/unit/org/apache/cassandra/cql3/validation/operations/DeleteTest.java ---------------------------------------------------------------------- diff --git a/test/unit/org/apache/cassandra/cql3/validation/operations/DeleteTest.java b/test/unit/org/apache/cassandra/cql3/validation/operations/DeleteTest.java index 9f770a5..4c9f4d6 100644 --- a/test/unit/org/apache/cassandra/cql3/validation/operations/DeleteTest.java +++ b/test/unit/org/apache/cassandra/cql3/validation/operations/DeleteTest.java @@ -43,15 +43,89 @@ public class DeleteTest extends CQLTester @Test public void testRangeDeletion() throws Throwable { - createTable("CREATE TABLE %s (a int, b int, c int, d int, PRIMARY KEY (a, b, c))"); + testRangeDeletion(true, true); + testRangeDeletion(false, true); + testRangeDeletion(true, false); + testRangeDeletion(false, false); + } + private void testRangeDeletion(boolean flushData, boolean flushTombstone) throws Throwable + { + createTable("CREATE TABLE %s (a int, b int, c int, d int, PRIMARY KEY (a, b, c))"); execute("INSERT INTO %s (a, b, c, d) VALUES (?, ?, ?, ?)", 1, 1, 1, 1); - flush(); + flush(flushData); execute("DELETE FROM %s WHERE a=? AND b=?", 1, 1); - flush(); + flush(flushTombstone); assertEmpty(execute("SELECT * FROM %s WHERE a=? AND b=? AND c=?", 1, 1, 1)); } + @Test + public void testDeleteRange() throws Throwable + { + testDeleteRange(true, true); + testDeleteRange(false, true); + testDeleteRange(true, false); + testDeleteRange(false, false); + } + + private void testDeleteRange(boolean flushData, boolean flushTombstone) throws Throwable + { + createTable("CREATE TABLE %s (a int, b int, c int, PRIMARY KEY (a, b))"); + + execute("INSERT INTO %s (a, b, c) VALUES (?, ?, ?)", 1, 1, 1); + execute("INSERT INTO %s (a, b, c) VALUES (?, ?, ?)", 2, 1, 2); + execute("INSERT INTO %s (a, b, c) VALUES (?, ?, ?)", 2, 2, 3); + execute("INSERT INTO %s (a, b, c) VALUES (?, ?, ?)", 2, 3, 4); + flush(flushData); + + execute("DELETE FROM %s WHERE a = ? AND b >= ?", 2, 2); + flush(flushTombstone); + + assertRowsIgnoringOrder(execute("SELECT * FROM %s"), + row(1, 1, 1), + row(2, 1, 2)); + + assertRows(execute("SELECT * FROM %s WHERE a = ? AND b = ?", 2, 1), + row(2, 1, 2)); + assertEmpty(execute("SELECT * FROM %s WHERE a = ? AND b = ?", 2, 2)); + assertEmpty(execute("SELECT * FROM %s WHERE a = ? AND b = ?", 2, 3)); + } + + @Test + public void testCrossMemSSTableMultiColumn() throws Throwable + { + testCrossMemSSTableMultiColumn(true, true); + testCrossMemSSTableMultiColumn(false, true); + testCrossMemSSTableMultiColumn(true, false); + testCrossMemSSTableMultiColumn(false, false); + } + + private void testCrossMemSSTableMultiColumn(boolean flushData, boolean flushTombstone) throws Throwable + { + createTable("CREATE TABLE %s (a int, b int, c int, PRIMARY KEY (a, b))"); + + execute("INSERT INTO %s (a, b, c) VALUES (?, ?, ?)", 1, 1, 1); + execute("INSERT INTO %s (a, b, c) VALUES (?, ?, ?)", 2, 1, 2); + execute("INSERT INTO %s (a, b, c) VALUES (?, ?, ?)", 2, 2, 2); + execute("INSERT INTO %s (a, b, c) VALUES (?, ?, ?)", 2, 3, 3); + flush(flushData); + + execute("DELETE FROM %s WHERE a = ? AND (b) = (?)", 2, 2); + execute("DELETE FROM %s WHERE a = ? AND (b) = (?)", 2, 3); + + flush(flushTombstone); + + assertRowsIgnoringOrder(execute("SELECT * FROM %s"), + row(1, 1, 1), + row(2, 1, 2)); + + assertRows(execute("SELECT * FROM %s WHERE a = ? AND b = ?", 2, 1), + row(2, 1, 2)); + assertEmpty(execute("SELECT * FROM %s WHERE a = ? AND b = ?", 2, 2)); + assertEmpty(execute("SELECT * FROM %s WHERE a = ? AND b = ?", 2, 3)); + } + + /** * Test simple deletion and in particular check for #4193 bug * migrated from cql_tests.py:TestCQL.deletion_test() @@ -440,7 +514,7 @@ public class DeleteTest extends CQLTester assertEmpty(execute("SELECT * FROM %s WHERE partitionKey = ? AND clustering = ?", 0, 1)); } - execute("DELETE FROM %s WHERE partitionKey = ? AND (clustering) = (?)", 0, 1); + execute("DELETE FROM %s WHERE partitionKey = ? AND clustering = ?", 0, 1); flush(forceFlush); assertEmpty(execute("SELECT value FROM %s WHERE partitionKey = ? AND clustering = ?", 0, 1)); http://git-wip-us.apache.org/repos/asf/cassandra/blob/2d6fd782/test/unit/org/apache/cassandra/db/partition/PartitionImplementationTest.java ---------------------------------------------------------------------- diff --git a/test/unit/org/apache/cassandra/db/partition/PartitionImplementationTest.java b/test/unit/org/apache/cassandra/db/partition/PartitionImplementationTest.java index f215331..f4c93d6 100644 --- a/test/unit/org/apache/cassandra/db/partition/PartitionImplementationTest.java +++ b/test/unit/org/apache/cassandra/db/partition/PartitionImplementationTest.java @@ -326,7 +326,7 @@ public class PartitionImplementationTest int pos = reversed ? KEY_RANGE : 0; int mul = reversed ? -1 : 1; boolean started = false; - while (searchIter.hasNext()) + while (pos < KEY_RANGE) { int skip = rand.nextInt(KEY_RANGE / 10); pos += skip * mul;