You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by sa...@apache.org on 2015/12/03 13:01:26 UTC

[03/10] cassandra git commit: Always verify expected tables in pseudo-system keyspaces at startup

Always verify expected tables in pseudo-system keyspaces at startup

Patch by Sam Tunnicliffe; reviewed by Sylvain Lebresne for
CASSANDRA-10761


Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo
Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/15f03ab4
Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/15f03ab4
Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/15f03ab4

Branch: refs/heads/cassandra-3.1
Commit: 15f03ab446854cf4d2999c3785d145c89bc3a3e4
Parents: e491f05
Author: Sam Tunnicliffe <sa...@beobal.com>
Authored: Wed Nov 25 16:45:02 2015 +0000
Committer: Sam Tunnicliffe <sa...@beobal.com>
Committed: Thu Dec 3 11:44:25 2015 +0000

----------------------------------------------------------------------
 CHANGES.txt                                      |  4 ++++
 src/java/org/apache/cassandra/config/Schema.java |  4 +++-
 .../apache/cassandra/db/ColumnFamilyStore.java   |  4 ++--
 src/java/org/apache/cassandra/db/Keyspace.java   | 12 ++++++------
 .../apache/cassandra/service/StorageService.java | 19 +++++--------------
 5 files changed, 20 insertions(+), 23 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cassandra/blob/15f03ab4/CHANGES.txt
----------------------------------------------------------------------
diff --git a/CHANGES.txt b/CHANGES.txt
index 787d145..12accac 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,3 +1,7 @@
+2.2.5
+ * Verify tables in pseudo-system keyspaces at startup (CASSANDRA-10761)
+
+
 2.2.4
  * Show CQL help in cqlsh in web browser (CASSANDRA-7225)
  * Serialize on disk the proper SSTable compression ratio (CASSANDRA-10775)

http://git-wip-us.apache.org/repos/asf/cassandra/blob/15f03ab4/src/java/org/apache/cassandra/config/Schema.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/config/Schema.java b/src/java/org/apache/cassandra/config/Schema.java
index 00c9358..110029e 100644
--- a/src/java/org/apache/cassandra/config/Schema.java
+++ b/src/java/org/apache/cassandra/config/Schema.java
@@ -525,8 +525,10 @@ public class Schema
         // since we're going to call initCf on the new one manually
         Keyspace.open(cfm.ksName);
 
+        // init the new CF before switching the KSM to the new one
+        // to avoid races as in CASSANDRA-10761
+        Keyspace.open(cfm.ksName).initCf(cfm, true);
         setKeyspaceDefinition(ksm);
-        Keyspace.open(ksm.name).initCf(cfm.cfId, cfm.cfName, true);
         MigrationManager.instance.notifyCreateColumnFamily(cfm);
     }
 

http://git-wip-us.apache.org/repos/asf/cassandra/blob/15f03ab4/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 8fefae2..5325c8a 100644
--- a/src/java/org/apache/cassandra/db/ColumnFamilyStore.java
+++ b/src/java/org/apache/cassandra/db/ColumnFamilyStore.java
@@ -506,9 +506,9 @@ public class ColumnFamilyStore implements ColumnFamilyStoreMBean
     }
 
 
-    public static ColumnFamilyStore createColumnFamilyStore(Keyspace keyspace, String columnFamily, boolean loadSSTables)
+    public static ColumnFamilyStore createColumnFamilyStore(Keyspace keyspace, CFMetaData metadata, boolean loadSSTables)
     {
-        return createColumnFamilyStore(keyspace, columnFamily, StorageService.getPartitioner(), Schema.instance.getCFMetaData(keyspace.getName(), columnFamily), loadSSTables);
+        return createColumnFamilyStore(keyspace, metadata.cfName, StorageService.getPartitioner(), metadata, loadSSTables);
     }
 
     public static synchronized ColumnFamilyStore createColumnFamilyStore(Keyspace keyspace,

http://git-wip-us.apache.org/repos/asf/cassandra/blob/15f03ab4/src/java/org/apache/cassandra/db/Keyspace.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/db/Keyspace.java b/src/java/org/apache/cassandra/db/Keyspace.java
index 92a0950..225369c 100644
--- a/src/java/org/apache/cassandra/db/Keyspace.java
+++ b/src/java/org/apache/cassandra/db/Keyspace.java
@@ -267,7 +267,7 @@ public class Keyspace
         for (CFMetaData cfm : new ArrayList<>(metadata.cfMetaData().values()))
         {
             logger.trace("Initializing {}.{}", getName(), cfm.cfName);
-            initCf(cfm.cfId, cfm.cfName, loadSSTables);
+            initCf(cfm, loadSSTables);
         }
     }
 
@@ -330,25 +330,25 @@ public class Keyspace
     /**
      * adds a cf to internal structures, ends up creating disk files).
      */
-    public void initCf(UUID cfId, String cfName, boolean loadSSTables)
+    public void initCf(CFMetaData metadata, boolean loadSSTables)
     {
-        ColumnFamilyStore cfs = columnFamilyStores.get(cfId);
+        ColumnFamilyStore cfs = columnFamilyStores.get(metadata.cfId);
 
         if (cfs == null)
         {
             // CFS being created for the first time, either on server startup or new CF being added.
             // We don't worry about races here; startup is safe, and adding multiple idential CFs
             // simultaneously is a "don't do that" scenario.
-            ColumnFamilyStore oldCfs = columnFamilyStores.putIfAbsent(cfId, ColumnFamilyStore.createColumnFamilyStore(this, cfName, loadSSTables));
+            ColumnFamilyStore oldCfs = columnFamilyStores.putIfAbsent(metadata.cfId, ColumnFamilyStore.createColumnFamilyStore(this, metadata, loadSSTables));
             // CFS mbean instantiation will error out before we hit this, but in case that changes...
             if (oldCfs != null)
-                throw new IllegalStateException("added multiple mappings for cf id " + cfId);
+                throw new IllegalStateException("added multiple mappings for cf id " + metadata.cfId);
         }
         else
         {
             // re-initializing an existing CF.  This will happen if you cleared the schema
             // on this node and it's getting repopulated from the rest of the cluster.
-            assert cfs.name.equals(cfName);
+            assert cfs.name.equals(metadata.cfName);
             cfs.metadata.reload();
             cfs.reload();
         }

http://git-wip-us.apache.org/repos/asf/cassandra/blob/15f03ab4/src/java/org/apache/cassandra/service/StorageService.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/service/StorageService.java b/src/java/org/apache/cassandra/service/StorageService.java
index fec3750..68e19c5 100644
--- a/src/java/org/apache/cassandra/service/StorageService.java
+++ b/src/java/org/apache/cassandra/service/StorageService.java
@@ -1026,18 +1026,6 @@ public class StorageService extends NotificationBroadcasterSupport implements IE
         MigrationManager.instance.register(new AuthMigrationListener());
     }
 
-    private void maybeAddTable(CFMetaData cfm)
-    {
-        try
-        {
-            MigrationManager.announceNewColumnFamily(cfm);
-        }
-        catch (AlreadyExistsException e)
-        {
-            logger.debug("Attempted to create new table {}, but it already exists", cfm.cfName);
-        }
-    }
-
     private void maybeAddKeyspace(KSMetaData ksm)
     {
         try
@@ -1064,14 +1052,17 @@ public class StorageService extends NotificationBroadcasterSupport implements IE
         // version of the schema, the one the node will be expecting.
 
         KSMetaData defined = Schema.instance.getKSMetaData(expected.name);
+        // If the keyspace doesn't exist, create it
         if (defined == null)
         {
-            // The keyspace doesn't exist, create it
             maybeAddKeyspace(expected);
-            return;
+            defined = Schema.instance.getKSMetaData(expected.name);
         }
 
         // While the keyspace exists, it might miss table or have outdated one
+        // There is also the potential for a race, as schema migrations add the bare
+        // keyspace into Schema.instance before adding its tables, so double check that
+        // all the expected tables are present
         for (CFMetaData expectedTable : expected.cfMetaData().values())
         {
             CFMetaData definedTable = defined.cfMetaData().get(expectedTable.cfName);