[cassandra] branch trunk updated: Fix clearing of legacy size_estimates

[mc...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

mck pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/cassandra.git


The following commit(s) were added to refs/heads/trunk by this push:
     new 0110311  Fix clearing of legacy size_estimates
0110311 is described below

commit 01103111ae08b51ccd18bb1c54ac60546546d9df
Author: David Capwell <dc...@apple.com>
AuthorDate: Wed Apr 29 12:31:27 2020 -0700

    Fix clearing of legacy size_estimates
    
     patch by David Capwell; reviewed by Eduard Tudenhöfner, Dinesh Joshi, Mick Semb Wever for CASSANDRA-15776
---
 CHANGES.txt                                        |  1 +
 .../org/apache/cassandra/db/SystemKeyspace.java    | 25 ++-----
 .../apache/cassandra/service/StorageService.java   | 19 +----
 .../distributed/test/TableEstimatesTest.java       | 84 ++++++++++++++++++++++
 .../apache/cassandra/tools/ClearSnapshotTest.java  |  4 ++
 5 files changed, 95 insertions(+), 38 deletions(-)

diff --git a/CHANGES.txt b/CHANGES.txt
index 98b3f68..f430f5c 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,4 +1,5 @@
 4.0-alpha5
+ * Fix clearing of legacy size_estimates (CASSANDRA-15776)
  * Update port when reconnecting to pre-4.0 SSL storage (CASSANDRA-15727)
  * Only calculate dynamicBadnessThreshold once per loop in DynamicEndpointSnitch (CASSANDRA-15798)
  * Cleanup redundant nodetool commands added in 4.0 (CASSANDRA-15256)
diff --git a/src/java/org/apache/cassandra/db/SystemKeyspace.java b/src/java/org/apache/cassandra/db/SystemKeyspace.java
index eb31f2d..655c7a0 100644
--- a/src/java/org/apache/cassandra/db/SystemKeyspace.java
+++ b/src/java/org/apache/cassandra/db/SystemKeyspace.java
@@ -1327,30 +1327,15 @@ public final class SystemKeyspace
     }
 
     /**
-     * Clears size estimates for a keyspace (used to manually clean when we miss a keyspace drop)
+     * truncates size_estimates and table_estimates tables
      */
-    public static void clearEstimates(String keyspace)
+    public static void clearAllEstimates()
     {
-        String cqlFormat = "DELETE FROM %s WHERE keyspace_name = ?";
-        String cql = String.format(cqlFormat, LegacySizeEstimates.toString());
-        executeInternal(cql, keyspace);
-        cql = String.format(cqlFormat, TableEstimates.toString());
-        executeInternal(cql, keyspace);
-    }
-
-    /**
-     * @return A multimap from keyspace to table for all tables with entries in size estimates
-     */
-    public static synchronized SetMultimap<String, String> getTablesWithSizeEstimates()
-    {
-        SetMultimap<String, String> keyspaceTableMap = HashMultimap.create();
-        String cql = String.format("SELECT keyspace_name, table_name FROM %s", TableEstimates.toString(), TABLE_ESTIMATES_TYPE_PRIMARY);
-        UntypedResultSet rs = executeInternal(cql);
-        for (UntypedResultSet.Row row : rs)
+        for (TableMetadata table : Arrays.asList(LegacySizeEstimates, TableEstimates))
         {
-            keyspaceTableMap.put(row.getString("keyspace_name"), row.getString("table_name"));
+            String cql = String.format("TRUNCATE TABLE " + table.toString());
+            executeInternal(cql);
         }
-        return keyspaceTableMap;
     }
 
     public static synchronized void updateAvailableRanges(String keyspace, Collection<Range<Token>> completedFullRanges, Collection<Range<Token>> completedTransientRanges)
diff --git a/src/java/org/apache/cassandra/service/StorageService.java b/src/java/org/apache/cassandra/service/StorageService.java
index 38da4e8..dfc1da9 100644
--- a/src/java/org/apache/cassandra/service/StorageService.java
+++ b/src/java/org/apache/cassandra/service/StorageService.java
@@ -3670,24 +3670,7 @@ public class StorageService extends NotificationBroadcasterSupport implements IE
 
     public void cleanupSizeEstimates()
     {
-        SetMultimap<String, String> sizeEstimates = SystemKeyspace.getTablesWithSizeEstimates();
-
-        for (Entry<String, Collection<String>> tablesByKeyspace : sizeEstimates.asMap().entrySet())
-        {
-            String keyspace = tablesByKeyspace.getKey();
-            if (!Schema.instance.getKeyspaces().contains(keyspace))
-            {
-                SystemKeyspace.clearEstimates(keyspace);
-            }
-            else
-            {
-                for (String table : tablesByKeyspace.getValue())
-                {
-                    if (Schema.instance.getTableMetadataRef(keyspace, table) == null)
-                        SystemKeyspace.clearEstimates(keyspace, table);
-                }
-            }
-        }
+        SystemKeyspace.clearAllEstimates();
     }
 
     /**
diff --git a/test/distributed/org/apache/cassandra/distributed/test/TableEstimatesTest.java b/test/distributed/org/apache/cassandra/distributed/test/TableEstimatesTest.java
new file mode 100644
index 0000000..0130f28
--- /dev/null
+++ b/test/distributed/org/apache/cassandra/distributed/test/TableEstimatesTest.java
@@ -0,0 +1,84 @@
+package org.apache.cassandra.distributed.test;
+
+import java.io.IOException;
+
+import org.junit.AfterClass;
+import org.junit.Assume;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+import org.apache.cassandra.distributed.Cluster;
+import org.apache.cassandra.distributed.api.ConsistencyLevel;
+import org.apache.cassandra.distributed.api.IInstance;
+import org.apache.cassandra.distributed.api.QueryResult;
+import org.assertj.core.api.Assertions;
+
+public class TableEstimatesTest extends TestBaseImpl
+{
+    private static Cluster CLUSTER;
+
+    @BeforeClass
+    public static void setupCluster() throws IOException
+    {
+        CLUSTER = init(Cluster.build(1).start());
+    }
+
+    @AfterClass
+    public static void teardownCluster()
+    {
+        if (CLUSTER != null)
+            CLUSTER.close();
+    }
+
+    /**
+     * Replaces Python Dtest: nodetool_test.py#test_refresh_size_estimates_clears_invalid_entries
+     */
+    @Test
+    public void refreshTableEstimatesClearsInvalidEntries()
+    {
+        String table_estimatesInsert = "INSERT INTO system.table_estimates (keyspace_name, table_name, range_type, range_start, range_end, mean_partition_size, partitions_count) VALUES (?, ?, ?, ?, ?, ?, ?)";
+        IInstance node = CLUSTER.get(1);
+
+        try
+        {
+            node.executeInternal(table_estimatesInsert, "system_auth", "bad_table", "local_primary", "-5", "5", 0L, 0L);
+            node.executeInternal(table_estimatesInsert, "bad_keyspace", "bad_table", "local_primary", "-5", "5", 0L, 0L);
+        }
+        catch (Exception e)
+        {
+            // to make this test portable (with the intent to extract out), handle the case where the table_estimates isn't defined
+            Assertions.assertThat(e.getClass().getCanonicalName()).isEqualTo("org.apache.cassandra.exceptions.InvalidRequestException");
+            Assertions.assertThat(e).hasMessageContaining("does not exist");
+            Assume.assumeTrue("system.table_estimates not present", false);
+        }
+
+        node.nodetoolResult("refreshsizeestimates").asserts().success();
+
+        QueryResult qr = CLUSTER.coordinator(1).executeWithResult("SELECT * FROM system.table_estimates WHERE keyspace_name=? AND table_name=?", ConsistencyLevel.ONE, "system_auth", "bad_table");
+        Assertions.assertThat(qr).isExhausted();
+
+        qr = CLUSTER.coordinator(1).executeWithResult("SELECT * FROM system.table_estimates WHERE keyspace_name=?", ConsistencyLevel.ONE, "bad_keyspace");
+        Assertions.assertThat(qr).isExhausted();
+    }
+
+    /**
+     * Replaces Python Dtest: nodetool_test.py#test_refresh_size_estimates_clears_invalid_entries
+     */
+    @Test
+    public void refreshSizeEstimatesClearsInvalidEntries()
+    {
+        String size_estimatesInsert = "INSERT INTO system.size_estimates (keyspace_name, table_name, range_start, range_end, mean_partition_size, partitions_count) VALUES (?, ?, ?, ?, ?, ?)";
+        IInstance node = CLUSTER.get(1);
+
+        node.executeInternal(size_estimatesInsert, "system_auth", "bad_table", "-5", "5", 0L, 0L);
+        node.executeInternal(size_estimatesInsert, "bad_keyspace", "bad_table", "-5", "5", 0L, 0L);
+
+        node.nodetoolResult("refreshsizeestimates").asserts().success();
+
+        QueryResult qr = CLUSTER.coordinator(1).executeWithResult("SELECT * FROM system.size_estimates WHERE keyspace_name=? AND table_name=?", ConsistencyLevel.ONE, "system_auth", "bad_table");
+        Assertions.assertThat(qr).isExhausted();
+
+        qr = CLUSTER.coordinator(1).executeWithResult("SELECT * FROM system.size_estimates WHERE keyspace_name=?", ConsistencyLevel.ONE, "bad_keyspace");
+        Assertions.assertThat(qr).isExhausted();
+    }
+}
diff --git a/test/unit/org/apache/cassandra/tools/ClearSnapshotTest.java b/test/unit/org/apache/cassandra/tools/ClearSnapshotTest.java
index 6b132ab..a73493b 100644
--- a/test/unit/org/apache/cassandra/tools/ClearSnapshotTest.java
+++ b/test/unit/org/apache/cassandra/tools/ClearSnapshotTest.java
@@ -29,6 +29,7 @@ import org.junit.BeforeClass;
 import org.junit.Test;
 
 import org.apache.cassandra.SchemaLoader;
+import org.apache.cassandra.config.DatabaseDescriptor;
 import org.apache.cassandra.service.EmbeddedCassandraService;
 
 public class ClearSnapshotTest extends ToolsTester
@@ -46,6 +47,9 @@ public class ClearSnapshotTest extends ToolsTester
         System.setProperty("cassandra.jmx.local.port", String.valueOf(JMX_PORT));
 
         SchemaLoader.prepareServer();
+        // CASSANDRA-15776 - size_estimates and table_estimates get truncated on startup, and auto snapshot is true by default for tests
+        // set it false so the test state doesn't see those snapshots
+        DatabaseDescriptor.setAutoSnapshot(false);
         cassandra = new EmbeddedCassandraService();
         cassandra.start();
 


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org