Repository: cassandra Updated Branches: refs/heads/cassandra-3.0 ee85507c2 -> c0886d877 refs/heads/cassandra-3.X 94a01f62a -> 3cb023355 refs/heads/trunk fc3b9bfd7 -> b921ddf2f
Fix for KeyCacheCqlTest flakiness patch by Stefania Alborghetti; reviewed by Robert Stupp for CASSANDRA-12801 Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/c0886d87 Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/c0886d87 Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/c0886d87 Branch: refs/heads/cassandra-3.0 Commit: c0886d87728a0f0a73b20edcd39373fa1458aba0 Parents: ee85507 Author: Stefania Alborghetti <stefania.alborghe...@datastax.com> Authored: Tue Oct 18 12:11:25 2016 +0800 Committer: Stefania Alborghetti <stefania.alborghe...@datastax.com> Committed: Tue Oct 25 09:53:47 2016 +0800 ---------------------------------------------------------------------- CHANGES.txt | 1 + .../apache/cassandra/schema/CachingParams.java | 4 +- .../org/apache/cassandra/cql3/CQLTester.java | 41 ++++++++--- .../apache/cassandra/cql3/KeyCacheCqlTest.java | 76 ++++++++++++++++---- 4 files changed, 98 insertions(+), 24 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cassandra/blob/c0886d87/CHANGES.txt ---------------------------------------------------------------------- diff --git a/CHANGES.txt b/CHANGES.txt index 6d38f83..9910245 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 3.0.10 + * Fix for KeyCacheCqlTest flakiness (CASSANDRA-12801) * Include SSTable filename in compacting large row message (CASSANDRA-12384) * Fix potential socket leak (CASSANDRA-12329, CASSANDRA-12330) * Fix ViewTest.testCompaction (CASSANDRA-12789) http://git-wip-us.apache.org/repos/asf/cassandra/blob/c0886d87/src/java/org/apache/cassandra/schema/CachingParams.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/schema/CachingParams.java b/src/java/org/apache/cassandra/schema/CachingParams.java index 2b5ab7c..1976835 100644 --- a/src/java/org/apache/cassandra/schema/CachingParams.java +++ b/src/java/org/apache/cassandra/schema/CachingParams.java @@ -20,6 +20,7 @@ package org.apache.cassandra.schema; import java.util.HashMap; import java.util.Map; +import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Objects; import com.google.common.collect.ImmutableMap; import org.apache.commons.lang3.StringUtils; @@ -53,7 +54,8 @@ public final class CachingParams public static final CachingParams CACHE_KEYS = new CachingParams(true, 0); public static final CachingParams CACHE_EVERYTHING = new CachingParams(true, Integer.MAX_VALUE); - static final CachingParams DEFAULT = new CachingParams(DEFAULT_CACHE_KEYS, DEFAULT_ROWS_PER_PARTITION_TO_CACHE); + @VisibleForTesting + public static CachingParams DEFAULT = new CachingParams(DEFAULT_CACHE_KEYS, DEFAULT_ROWS_PER_PARTITION_TO_CACHE); final boolean cacheKeys; final int rowsPerPartitionToCache; http://git-wip-us.apache.org/repos/asf/cassandra/blob/c0886d87/test/unit/org/apache/cassandra/cql3/CQLTester.java ---------------------------------------------------------------------- diff --git a/test/unit/org/apache/cassandra/cql3/CQLTester.java b/test/unit/org/apache/cassandra/cql3/CQLTester.java index 7f5eb02..3d45393 100644 --- a/test/unit/org/apache/cassandra/cql3/CQLTester.java +++ b/test/unit/org/apache/cassandra/cql3/CQLTester.java @@ -543,8 +543,13 @@ public abstract class CQLTester protected String createTable(String query) { + return createTable(KEYSPACE, query); + } + + protected String createTable(String keyspace, String query) + { String currentTable = createTableName(); - String fullQuery = formatQuery(query); + String fullQuery = formatQuery(keyspace, query); logger.info(fullQuery); schemaChange(fullQuery); return currentTable; @@ -581,16 +586,24 @@ public abstract class CQLTester protected void dropTable(String query) { - String fullQuery = String.format(query, KEYSPACE + "." + currentTable()); - logger.info(fullQuery); - schemaChange(fullQuery); + dropFormattedTable(String.format(query, KEYSPACE + "." + currentTable())); + } + + protected void dropFormattedTable(String formattedQuery) + { + logger.info(formattedQuery); + schemaChange(formattedQuery); } protected void createIndex(String query) { - String fullQuery = formatQuery(query); - logger.info(fullQuery); - schemaChange(fullQuery); + createFormattedIndex(formatQuery(query)); + } + + protected void createFormattedIndex(String formattedQuery) + { + logger.info(formattedQuery); + schemaChange(formattedQuery); } /** @@ -694,16 +707,24 @@ public abstract class CQLTester return sessions.get(protocolVersion); } - private String formatQuery(String query) + protected String formatQuery(String query) + { + return formatQuery(KEYSPACE, query); + } + + protected final String formatQuery(String keyspace, String query) { String currentTable = currentTable(); - return currentTable == null ? query : String.format(query, KEYSPACE + "." + currentTable); + return currentTable == null ? query : String.format(query, keyspace + "." + currentTable); } protected UntypedResultSet execute(String query, Object... values) throws Throwable { - query = formatQuery(query); + return executeFormattedQuery(formatQuery(query), values); + } + protected UntypedResultSet executeFormattedQuery(String query, Object... values) throws Throwable + { UntypedResultSet rs; if (usePrepared) { http://git-wip-us.apache.org/repos/asf/cassandra/blob/c0886d87/test/unit/org/apache/cassandra/cql3/KeyCacheCqlTest.java ---------------------------------------------------------------------- diff --git a/test/unit/org/apache/cassandra/cql3/KeyCacheCqlTest.java b/test/unit/org/apache/cassandra/cql3/KeyCacheCqlTest.java index 54d39b1..3f87343 100644 --- a/test/unit/org/apache/cassandra/cql3/KeyCacheCqlTest.java +++ b/test/unit/org/apache/cassandra/cql3/KeyCacheCqlTest.java @@ -24,6 +24,7 @@ import java.util.Iterator; import java.util.List; import org.junit.Assert; +import org.junit.BeforeClass; import org.junit.Test; import org.apache.cassandra.cache.KeyCacheKey; @@ -31,6 +32,7 @@ import org.apache.cassandra.config.Schema; import org.apache.cassandra.db.Keyspace; import org.apache.cassandra.metrics.CacheMetrics; import org.apache.cassandra.metrics.CassandraMetricsRegistry; +import org.apache.cassandra.schema.CachingParams; import org.apache.cassandra.service.CacheService; import org.apache.cassandra.service.StorageService; @@ -44,7 +46,7 @@ import org.apache.cassandra.utils.Pair; public class KeyCacheCqlTest extends CQLTester { - static final String commonColumnsDef = + private static final String commonColumnsDef = "part_key_a int," + "part_key_b text," + "clust_key_a int," + @@ -54,7 +56,7 @@ public class KeyCacheCqlTest extends CQLTester "col_int int," + "col_long bigint," + "col_blob blob,"; - static final String commonColumns = + private static final String commonColumns = "part_key_a," + "part_key_b," + "clust_key_a," + @@ -65,7 +67,8 @@ public class KeyCacheCqlTest extends CQLTester "col_long"; // 1200 chars - static final String longString = "0123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789" + + private static final String longString = + "0123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789" + "0123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789" + "0123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789" + "0123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789" + @@ -78,6 +81,53 @@ public class KeyCacheCqlTest extends CQLTester "0123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789" + "0123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789"; + /** + * Prevent system tables from populating the key cache to ensure that + * the test can reliably check the size of the key cache size and its metrics. + * Test tables will be created with caching enabled manually in the CQL statement, + * see {@link KeyCacheCqlTest#createTable(String)}. + * + * Then call the base class initialization, which must be done after disabling the key cache. + */ + @BeforeClass + public static void setUpClass() + { + CachingParams.DEFAULT = CachingParams.CACHE_NOTHING; + CQLTester.setUpClass(); + } + + /** + * Create a table in KEYSPACE_PER_TEST_PER_TEST because it will get dropped synchronously by CQLTester after + * each test, whereas the default keyspace gets dropped asynchronously and this may cause unexpected + * flush operations during a test, which would change the expected result of metrics. + * + * Then add manual caching, since by default we have disabled cachinng for all other tables, to ensure + * that we can assert on the key cache size and metrics. + */ + @Override + protected String createTable(String query) + { + return super.createTable(KEYSPACE_PER_TEST, query + " WITH caching = { 'keys' : 'ALL', 'rows_per_partition' : '0' }"); + } + + @Override + protected UntypedResultSet execute(String query, Object... values) throws Throwable + { + return executeFormattedQuery(formatQuery(KEYSPACE_PER_TEST, query), values); + } + + @Override + protected void createIndex(String query) + { + createFormattedIndex(formatQuery(KEYSPACE_PER_TEST, query)); + } + + @Override + protected void dropTable(String query) + { + dropFormattedTable(String.format(query, KEYSPACE_PER_TEST + "." + currentTable())); + } + @Test public void testSliceQueries() throws Throwable { @@ -96,7 +146,7 @@ public class KeyCacheCqlTest extends CQLTester } } - StorageService.instance.forceKeyspaceFlush(KEYSPACE); + StorageService.instance.forceKeyspaceFlush(KEYSPACE_PER_TEST); for (int pkInt = 0; pkInt < 20; pkInt++) { @@ -225,7 +275,7 @@ public class KeyCacheCqlTest extends CQLTester //Test Schema.getColumnFamilyStoreIncludingIndexes, several null check paths //are defensive and unreachable assertNull(Schema.instance.getColumnFamilyStoreIncludingIndexes(Pair.create("foo", "bar"))); - assertNull(Schema.instance.getColumnFamilyStoreIncludingIndexes(Pair.create(KEYSPACE, "bar"))); + assertNull(Schema.instance.getColumnFamilyStoreIncludingIndexes(Pair.create(KEYSPACE_PER_TEST, "bar"))); dropTable("DROP TABLE %s"); Schema.instance.updateVersion(); @@ -294,7 +344,7 @@ public class KeyCacheCqlTest extends CQLTester while(iter.hasNext()) { KeyCacheKey key = iter.next(); - Assert.assertFalse(key.ksAndCFName.left.equals("KEYSPACE")); + Assert.assertFalse(key.ksAndCFName.left.equals("KEYSPACE_PER_TEST")); Assert.assertFalse(key.ksAndCFName.right.startsWith(table)); } } @@ -406,8 +456,8 @@ public class KeyCacheCqlTest extends CQLTester prepareTable(table); if (index != null) { - StorageService.instance.disableAutoCompaction(KEYSPACE, table + '.' + index); - Keyspace.open(KEYSPACE).getColumnFamilyStore(table).indexManager.getIndexByName(index).getBlockingFlushTask().call(); + StorageService.instance.disableAutoCompaction(KEYSPACE_PER_TEST, table + '.' + index); + Keyspace.open(KEYSPACE_PER_TEST).getColumnFamilyStore(table).indexManager.getIndexByName(index).getBlockingFlushTask().call(); } for (int i = 0; i < 100; i++) @@ -430,18 +480,18 @@ public class KeyCacheCqlTest extends CQLTester if (i % 10 == 9) { - Keyspace.open(KEYSPACE).getColumnFamilyStore(table).forceFlush().get(); + Keyspace.open(KEYSPACE_PER_TEST).getColumnFamilyStore(table).forceFlush().get(); if (index != null) - Keyspace.open(KEYSPACE).getColumnFamilyStore(table).indexManager.getIndexByName(index).getBlockingFlushTask().call(); + Keyspace.open(KEYSPACE_PER_TEST).getColumnFamilyStore(table).indexManager.getIndexByName(index).getBlockingFlushTask().call(); } } } private static void prepareTable(String table) throws IOException, InterruptedException, java.util.concurrent.ExecutionException { - StorageService.instance.disableAutoCompaction(KEYSPACE, table); - Keyspace.open(KEYSPACE).getColumnFamilyStore(table).forceFlush().get(); - Keyspace.open(KEYSPACE).getColumnFamilyStore(table).truncateBlocking(); + StorageService.instance.disableAutoCompaction(KEYSPACE_PER_TEST, table); + Keyspace.open(KEYSPACE_PER_TEST).getColumnFamilyStore(table).forceFlush().get(); + Keyspace.open(KEYSPACE_PER_TEST).getColumnFamilyStore(table).truncateBlocking(); } private static List<String> makeList(String value)
