cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From stefa...@apache.org
Subject [2/6] cassandra git commit: Fix for KeyCacheCqlTest flakiness
Date Tue, 25 Oct 2016 01:59:51 GMT
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 <stefania.alborghetti@datastax.com>
Authored: Tue Oct 18 12:11:25 2016 +0800
Committer: Stefania Alborghetti <stefania.alborghetti@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)


Mime
View raw message