You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by st...@apache.org on 2016/10/25 01:59:51 UTC
[2/6] cassandra git commit: Fix for KeyCacheCqlTest flakiness
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.X
Commit: c0886d87728a0f0a73b20edcd39373fa1458aba0
Parents: ee85507
Author: Stefania Alborghetti <st...@datastax.com>
Authored: Tue Oct 18 12:11:25 2016 +0800
Committer: Stefania Alborghetti <st...@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)