cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From alek...@apache.org
Subject [1/3] cassandra git commit: Fix counting of tombstones for TombstoneOverwhelmingException
Date Tue, 12 May 2015 18:06:39 GMT
Repository: cassandra
Updated Branches:
  refs/heads/trunk 5ff69f2c9 -> 21a915cd7


Fix counting of tombstones for TombstoneOverwhelmingException

patch by Aleksey Yeschenko; reviewed by Tyler Hobbs for CASSANDRA-9299


Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo
Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/bed42c21
Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/bed42c21
Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/bed42c21

Branch: refs/heads/trunk
Commit: bed42c2104ebaac83da4292703c08a5c963e062c
Parents: a7cae32
Author: Aleksey Yeschenko <aleksey@apache.org>
Authored: Tue May 12 20:52:13 2015 +0300
Committer: Aleksey Yeschenko <aleksey@apache.org>
Committed: Tue May 12 20:52:13 2015 +0300

----------------------------------------------------------------------
 CHANGES.txt                                     |   1 +
 .../apache/cassandra/db/ColumnFamilyStore.java  |   2 +-
 .../cassandra/db/filter/ColumnCounter.java      |  33 ++--
 .../cassandra/db/filter/SliceQueryFilter.java   |  19 ++-
 .../SliceQueryFilterWithTombstonesTest.java     | 150 +++++++++++++++++++
 5 files changed, 182 insertions(+), 23 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cassandra/blob/bed42c21/CHANGES.txt
----------------------------------------------------------------------
diff --git a/CHANGES.txt b/CHANGES.txt
index 685b945..d7d01cf 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,4 +1,5 @@
 2.0.15:
+ * Fix counting of tombstones for TombstoneOverwhelmingException (CASSANDRA-9299)
  * Fix ReconnectableSnitch reconnecting to peers during upgrade (CASSANDRA-6702)
  * Include keyspace and table name in error log for collections over the size
    limit (CASSANDRA-9286)

http://git-wip-us.apache.org/repos/asf/cassandra/blob/bed42c21/src/java/org/apache/cassandra/db/ColumnFamilyStore.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/db/ColumnFamilyStore.java b/src/java/org/apache/cassandra/db/ColumnFamilyStore.java
index d8640e8..5ea1287 100644
--- a/src/java/org/apache/cassandra/db/ColumnFamilyStore.java
+++ b/src/java/org/apache/cassandra/db/ColumnFamilyStore.java
@@ -1396,7 +1396,7 @@ public class ColumnFamilyStore implements ColumnFamilyStoreMBean
             if (filter.filter instanceof SliceQueryFilter)
             {
                 // Log the number of tombstones scanned on single key queries
-                metric.tombstoneScannedHistogram.update(((SliceQueryFilter) filter.filter).lastIgnored());
+                metric.tombstoneScannedHistogram.update(((SliceQueryFilter) filter.filter).lastTombstones());
                 metric.liveScannedHistogram.update(((SliceQueryFilter) filter.filter).lastLive());
             }
         }

http://git-wip-us.apache.org/repos/asf/cassandra/blob/bed42c21/src/java/org/apache/cassandra/db/filter/ColumnCounter.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/db/filter/ColumnCounter.java b/src/java/org/apache/cassandra/db/filter/ColumnCounter.java
index 814d8ed..2d0df1f 100644
--- a/src/java/org/apache/cassandra/db/filter/ColumnCounter.java
+++ b/src/java/org/apache/cassandra/db/filter/ColumnCounter.java
@@ -31,7 +31,7 @@ import org.apache.cassandra.utils.ByteBufferUtil;
 public class ColumnCounter
 {
     protected int live;
-    protected int ignored;
+    protected int tombstones;
     protected final long timestamp;
 
     public ColumnCounter(long timestamp)
@@ -41,15 +41,15 @@ public class ColumnCounter
 
     public void count(Column column, DeletionInfo.InOrderTester tester)
     {
-        if (!isLive(column, tester, timestamp))
-            ignored++;
-        else
-            live++;
-    }
+        // The cell is shadowed by a higher-level deletion, and won't be retained.
+        // For the purposes of this counter, we don't care if it's a tombstone or not.
+        if (tester.isDeleted(column))
+            return;
 
-    protected static boolean isLive(Column column, DeletionInfo.InOrderTester tester, long
timestamp)
-    {
-        return column.isLive(timestamp) && (!tester.isDeleted(column));
+        if (column.isLive(timestamp))
+            live++;
+        else
+            tombstones++;
     }
 
     public int live()
@@ -57,9 +57,9 @@ public class ColumnCounter
         return live;
     }
 
-    public int ignored()
+    public int tombstones()
     {
-        return ignored;
+        return tombstones;
     }
 
     public ColumnCounter countAll(ColumnFamily container)
@@ -101,9 +101,12 @@ public class ColumnCounter
 
         public void count(Column column, DeletionInfo.InOrderTester tester)
         {
-            if (!isLive(column, tester, timestamp))
+            if (tester.isDeleted(column))
+                return;
+
+            if (!column.isLive(timestamp))
             {
-                ignored++;
+                tombstones++;
                 return;
             }
 
@@ -119,11 +122,11 @@ public class ColumnCounter
             if (previous == null)
             {
                 // Only the first group can be static
-                previousGroupIsStatic = type.isStaticName(column.name());
+                previousGroupIsStatic = CompositeType.isStaticName(column.name());
             }
             else
             {
-                boolean isSameGroup = previousGroupIsStatic == type.isStaticName(column.name());
+                boolean isSameGroup = previousGroupIsStatic == CompositeType.isStaticName(column.name());
                 if (isSameGroup)
                 {
                     for (int i = 0; i < toGroup; i++)

http://git-wip-us.apache.org/repos/asf/cassandra/blob/bed42c21/src/java/org/apache/cassandra/db/filter/SliceQueryFilter.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/db/filter/SliceQueryFilter.java b/src/java/org/apache/cassandra/db/filter/SliceQueryFilter.java
index ad1a92b..6e6ab6b 100644
--- a/src/java/org/apache/cassandra/db/filter/SliceQueryFilter.java
+++ b/src/java/org/apache/cassandra/db/filter/SliceQueryFilter.java
@@ -200,7 +200,7 @@ public class SliceQueryFilter implements IDiskAtomFilter
             if (columnCounter.live() > count)
                 break;
 
-            if (respectTombstoneThresholds() && columnCounter.ignored() > DatabaseDescriptor.getTombstoneFailureThreshold())
+            if (respectTombstoneThresholds() && columnCounter.tombstones() > DatabaseDescriptor.getTombstoneFailureThreshold())
             {
                 Tracing.trace("Scanned over {} tombstones; query aborted (see tombstone_failure_threshold)",
DatabaseDescriptor.getTombstoneFailureThreshold());
                 logger.error("Scanned over {} tombstones in {}.{}; query aborted (see tombstone_failure_threshold)",
@@ -211,8 +211,8 @@ public class SliceQueryFilter implements IDiskAtomFilter
             container.addIfRelevant(column, tester, gcBefore);
         }
 
-        Tracing.trace("Read {} live and {} tombstoned cells", columnCounter.live(), columnCounter.ignored());
-        if (respectTombstoneThresholds() && columnCounter.ignored() > DatabaseDescriptor.getTombstoneWarnThreshold())
+        Tracing.trace("Read {} live and {} tombstone cells", columnCounter.live(), columnCounter.tombstones());
+        if (respectTombstoneThresholds() && columnCounter.tombstones() > DatabaseDescriptor.getTombstoneWarnThreshold())
         {
             StringBuilder sb = new StringBuilder();
             AbstractType<?> type = container.metadata().comparator;
@@ -228,8 +228,13 @@ public class SliceQueryFilter implements IDiskAtomFilter
                 sb.append(']');
             }
 
-            logger.warn("Read {} live and {} tombstoned cells in {}.{} (see tombstone_warn_threshold).
{} columns was requested, slices={}",
-                        columnCounter.live(), columnCounter.ignored(), container.metadata().ksName,
container.metadata().cfName, count, sb);
+            logger.warn("Read {} live and {} tombstone cells in {}.{} (see tombstone_warn_threshold).
{} columns was requested, slices={}",
+                        columnCounter.live(),
+                        columnCounter.tombstones(),
+                        container.metadata().ksName,
+                        container.metadata().cfName,
+                        count,
+                        sb);
         }
     }
 
@@ -305,9 +310,9 @@ public class SliceQueryFilter implements IDiskAtomFilter
         return columnCounter == null ? 0 : Math.min(columnCounter.live(), count);
     }
 
-    public int lastIgnored()
+    public int lastTombstones()
     {
-        return columnCounter == null ? 0 : columnCounter.ignored();
+        return columnCounter == null ? 0 : columnCounter.tombstones();
     }
 
     public int lastLive()

http://git-wip-us.apache.org/repos/asf/cassandra/blob/bed42c21/test/unit/org/apache/cassandra/cql3/SliceQueryFilterWithTombstonesTest.java
----------------------------------------------------------------------
diff --git a/test/unit/org/apache/cassandra/cql3/SliceQueryFilterWithTombstonesTest.java b/test/unit/org/apache/cassandra/cql3/SliceQueryFilterWithTombstonesTest.java
new file mode 100644
index 0000000..4440782
--- /dev/null
+++ b/test/unit/org/apache/cassandra/cql3/SliceQueryFilterWithTombstonesTest.java
@@ -0,0 +1,150 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.cassandra.cql3;
+
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+import org.apache.cassandra.SchemaLoader;
+import org.apache.cassandra.config.DatabaseDescriptor;
+import org.apache.cassandra.db.ConsistencyLevel;
+import org.apache.cassandra.db.filter.TombstoneOverwhelmingException;
+import org.apache.cassandra.gms.Gossiper;
+
+import static junit.framework.Assert.assertTrue;
+import static junit.framework.Assert.fail;
+
+import static org.apache.cassandra.cql3.QueryProcessor.process;
+import static org.apache.cassandra.cql3.QueryProcessor.processInternal;
+
+/**
+ * Test that TombstoneOverwhelmingException gets thrown when it should be and doesn't when
it shouldn't be.
+ */
+public class SliceQueryFilterWithTombstonesTest
+{
+    static final String KEYSPACE = "tombstone_overwhelming_exception_test";
+    static final String TABLE = "overwhelmed";
+
+    static final int ORIGINAL_THRESHOLD = DatabaseDescriptor.getTombstoneFailureThreshold();
+    static final int THRESHOLD = 100;
+
+    @BeforeClass
+    public static void setUp() throws Throwable
+    {
+        SchemaLoader.loadSchema();
+
+        process(String.format("CREATE KEYSPACE IF NOT EXISTS %s WITH replication = {'class':
'SimpleStrategy', 'replication_factor': '1'}",
+                              KEYSPACE),
+                ConsistencyLevel.ONE);
+
+        process(String.format("CREATE TABLE IF NOT EXISTS %s.%s (a text, b text, c text,
PRIMARY KEY (a, b));",
+                              KEYSPACE,
+                              TABLE),
+                ConsistencyLevel.ONE);
+
+        DatabaseDescriptor.setTombstoneFailureThreshold(THRESHOLD);
+    }
+
+    @AfterClass
+    public static void tearDown()
+    {
+        Gossiper.instance.stop();
+
+        DatabaseDescriptor.setTombstoneFailureThreshold(ORIGINAL_THRESHOLD);
+    }
+
+    private static UntypedResultSet execute(String query)
+    {
+        return processInternal(String.format(query, KEYSPACE, TABLE));
+    }
+
+    @Test
+    public void testBelowThresholdSelect()
+    {
+        // insert exactly the amount of tombstones that shouldn't trigger an exception
+        for (int i = 0; i < THRESHOLD; i++)
+            execute("INSERT INTO %s.%s (a, b, c) VALUES ('key1', 'column" + i + "', null);");
+
+        try
+        {
+            execute("SELECT * FROM %s.%s WHERE a = 'key1';");
+        }
+        catch (Throwable e)
+        {
+            fail("SELECT with tombstones below the threshold should not have failed, but
has: " + e);
+        }
+    }
+
+    @Test
+    public void testBeyondThresholdSelect()
+    {
+        // insert exactly the amount of tombstones that *SHOULD* trigger an exception
+        for (int i = 0; i < THRESHOLD + 1; i++)
+            execute("INSERT INTO %s.%s (a, b, c) VALUES ('key2', 'column" + i + "', null);");
+
+        try
+        {
+            execute("SELECT * FROM %s.%s WHERE a = 'key2';");
+            fail("SELECT with tombstones beyond the threshold should have failed, but hasn't");
+        }
+        catch (Throwable e)
+        {
+            assertTrue(e instanceof TombstoneOverwhelmingException);
+        }
+    }
+
+    @Test
+    public void testAllShadowedSelect()
+    {
+        // insert exactly the amount of tombstones that *SHOULD* normally trigger an exception
+        for (int i = 0; i < THRESHOLD + 1; i++)
+            execute("INSERT INTO %s.%s (a, b, c) VALUES ('key3', 'column" + i + "', null);");
+
+        // delete all with a partition level tombstone
+        execute("DELETE FROM %s.%s WHERE a = 'key3'");
+
+        try
+        {
+            execute("SELECT * FROM %s.%s WHERE a = 'key3';");
+        }
+        catch (Throwable e)
+        {
+            fail("SELECT with tombstones shadowed by a partition tombstone should not have
failed, but has: " + e);
+        }
+    }
+
+    @Test
+    public void testLiveShadowedCellsSelect()
+    {
+        for (int i = 0; i < THRESHOLD + 1; i++)
+            execute("INSERT INTO %s.%s (a, b, c) VALUES ('key4', 'column" + i + "', 'column');");
+
+        // delete all with a partition level tombstone
+        execute("DELETE FROM %s.%s WHERE a = 'key4'");
+
+        try
+        {
+            execute("SELECT * FROM %s.%s WHERE a = 'key4';");
+        }
+        catch (Throwable e)
+        {
+            fail("SELECT with regular cells shadowed by a partition tombstone should not
have failed, but has: " + e);
+        }
+    }
+}


Mime
View raw message