Return-Path: X-Original-To: apmail-cassandra-commits-archive@www.apache.org Delivered-To: apmail-cassandra-commits-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id B38FB1854B for ; Tue, 12 May 2015 18:06:39 +0000 (UTC) Received: (qmail 25513 invoked by uid 500); 12 May 2015 18:06:39 -0000 Delivered-To: apmail-cassandra-commits-archive@cassandra.apache.org Received: (qmail 25476 invoked by uid 500); 12 May 2015 18:06:39 -0000 Mailing-List: contact commits-help@cassandra.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@cassandra.apache.org Delivered-To: mailing list commits@cassandra.apache.org Received: (qmail 25463 invoked by uid 99); 12 May 2015 18:06:39 -0000 Received: from git1-us-west.apache.org (HELO git1-us-west.apache.org) (140.211.11.23) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 12 May 2015 18:06:39 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id 52340E17DD; Tue, 12 May 2015 18:06:39 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: aleksey@apache.org To: commits@cassandra.apache.org Date: Tue, 12 May 2015 18:06:39 -0000 Message-Id: X-Mailer: ASF-Git Admin Mailer Subject: [1/3] cassandra git commit: Fix counting of tombstones for TombstoneOverwhelmingException 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 Authored: Tue May 12 20:52:13 2015 +0300 Committer: Aleksey Yeschenko 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); + } + } +}