You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by al...@apache.org on 2015/05/12 19:52:49 UTC
cassandra git commit: Fix counting of tombstones for
TombstoneOverwhelmingException
Repository: cassandra
Updated Branches:
refs/heads/cassandra-2.0 a7cae3255 -> bed42c210
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/cassandra-2.0
Commit: bed42c2104ebaac83da4292703c08a5c963e062c
Parents: a7cae32
Author: Aleksey Yeschenko <al...@apache.org>
Authored: Tue May 12 20:52:13 2015 +0300
Committer: Aleksey Yeschenko <al...@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);
+ }
+ }
+}