cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ifesdj...@apache.org
Subject [1/5] cassandra git commit: Make reading of range tombstones more reliable
Date Fri, 07 Apr 2017 10:21:33 GMT
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.petrov@gmail.com>
Authored: Fri Apr 7 12:09:32 2017 +0200
Committer: Alex Petrov <oleksandr.petrov@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;


Mime
View raw message