Merge branch 'cassandra-3.11' into trunk

CHANGES.txt

@@ -10,6 +10,7 @@
  * TLS connections to the storage port on a node without server encryption configured causes java.io.IOException accessing missing keystore (CASSANDRA-16144)
 Merged from 3.11:
 Merged from 3.0:
+ * Improved check of num_tokens against the length of initial_token (CASSANDRA-14477)
  * Fix a race condition on ColumnFamilyStore and TableMetrics (CASSANDRA-16228)
  * Remove the SEPExecutor blocking behavior (CASSANDRA-16186)
  * Wait for schema agreement when bootstrapping (CASSANDRA-15158)

NEWS.txt

@@ -256,6 +256,9 @@ Upgrading
       sstables already added/removed - see CASSANDRA-14103 for details.
     - Support for JNA with glibc 2.6 and earlier has been removed. Centos 5, Debian 4, and Ubuntu 7.10 operating systems
       must be first upgraded. See CASSANDRA-16212 for more.
+    - In cassandra.yaml, num_tokens must be defined if initial_token is defined.
+      If it is not defined, or not equal to the numbers of tokens defined in initial_tokens,
+      the node will not start. See CASSANDRA-14477 for details.
 
 
 Deprecation

src/java/org/apache/cassandra/config/Config.java

@@ -83,7 +83,7 @@ public class Config
 
     /* initial token in the ring */
     public String initial_token;
-    public int num_tokens = 1;
+    public Integer num_tokens;
     /** Triggers automatic allocation of tokens if set, using the replication strategy of the referenced keyspace */
     public String allocate_tokens_for_keyspace = null;
     /** Triggers automatic allocation of tokens if set, based on the provided replica count for a datacenter */

src/java/org/apache/cassandra/config/DatabaseDescriptor.java

@@ -354,7 +354,7 @@ public class DatabaseDescriptor
 
         applySnitch();
 
-        applyInitialTokens();
+        applyTokensConfig();
 
         applySeedProvider();
 
@@ -680,7 +680,7 @@ public class DatabaseDescriptor
         if (conf.concurrent_materialized_view_builders <= 0)
             throw new ConfigurationException("concurrent_materialized_view_builders should be strictly greater than 0, but was " + conf.concurrent_materialized_view_builders, false);
 
-        if (conf.num_tokens > MAX_NUM_TOKENS)
+        if (conf.num_tokens != null && conf.num_tokens > MAX_NUM_TOKENS)
             throw new ConfigurationException(String.format("A maximum number of %d tokens per node is supported", MAX_NUM_TOKENS), false);
 
         try
@@ -1075,17 +1075,36 @@ public class DatabaseDescriptor
         logger.info("found {}::{} less than lowest acceptable value {}, continuing with {}", property, actualValue, lowestAcceptedValue, lowestAcceptedValue);
     }
 
-    public static void applyInitialTokens()
+    public static void applyTokensConfig()
+    {
+        applyTokensConfig(conf);
+    }
+
+    static void applyTokensConfig(Config conf)
     {
         if (conf.initial_token != null)
         {
             Collection<String> tokens = tokensFromString(conf.initial_token);
+            if (conf.num_tokens == null)
+            {
+                throw new ConfigurationException("initial_token was set but num_tokens is not!", false);
+            }
+
             if (tokens.size() != conf.num_tokens)
-                throw new ConfigurationException("The number of initial tokens (by initial_token) specified is different from num_tokens value", false);
+            {
+                throw new ConfigurationException(String.format("The number of initial tokens (by initial_token) specified (%s) is different from num_tokens value (%s)",
+                                                               tokens.size(),
+                                                               conf.num_tokens),
+                                                 false);
+            }
 
             for (String token : tokens)
                 partitioner.getTokenFactory().validate(token);
         }
+        else if (conf.num_tokens == null)
+        {
+            conf.num_tokens = 1;
+        }
     }
 
     // definitely not safe for tools + clients - implicitly instantiates StorageService

test/unit/org/apache/cassandra/config/DatabaseDescriptorRefTest.java

@@ -257,7 +257,7 @@ public class DatabaseDescriptorRefTest
         for (String methodName : new String[]{
             "clientInitialization",
             "applyAddressConfig",
-            "applyInitialTokens",
+            "applyTokensConfig",
             // no seed provider in default configuration for clients
             // "applySeedProvider",
             // definitely not safe for clients - implicitly instantiates schema

test/unit/org/apache/cassandra/config/DatabaseDescriptorTest.java

@@ -502,4 +502,82 @@ public class DatabaseDescriptorTest
         DatabaseDescriptor.applyRepairCommandPoolSize(conf);
         assertThat(conf.repair_command_pool_size).isEqualTo(conf.concurrent_validations + 1);
     }
+
+    @Test
+    public void testApplyInitialTokensInitialTokensSetNumTokensSetAndDoesMatch()
+    {
+        Config config = DatabaseDescriptor.loadConfig();
+        config.initial_token = "0,256,1024";
+        config.num_tokens = 3;
+
+        try
+        {
+            DatabaseDescriptor.applyTokensConfig(config);
+            Assert.assertEquals(Integer.valueOf(3), config.num_tokens);
+            Assert.assertEquals(3, DatabaseDescriptor.tokensFromString(config.initial_token).size());
+        }
+        catch (ConfigurationException e)
+        {
+            Assert.fail("number of tokens in initial_token=0,256,1024 does not match num_tokens = 3");
+        }
+    }
+
+    @Test
+    public void testApplyInitialTokensInitialTokensSetNumTokensSetAndDoesntMatch()
+    {
+        Config config = DatabaseDescriptor.loadConfig();
+        config.initial_token = "0,256,1024";
+        config.num_tokens = 10;
+
+        try
+        {
+            DatabaseDescriptor.applyTokensConfig(config);
+
+            Assert.fail("initial_token = 0,256,1024 and num_tokens = 10 but applyInitialTokens() did not fail!");
+        }
+        catch (ConfigurationException ex)
+        {
+            Assert.assertEquals("The number of initial tokens (by initial_token) specified (3) is different from num_tokens value (10)",
+                                ex.getMessage());
+        }
+    }
+
+    @Test
+    public void testApplyInitialTokensInitialTokensSetNumTokensNotSet()
+    {
+        Config config = DatabaseDescriptor.loadConfig();
+        config.initial_token = "0,256,1024";
+
+        try
+        {
+            DatabaseDescriptor.applyTokensConfig(config);
+            Assert.fail("setting initial_token and not setting num_tokens is invalid");
+        }
+        catch (ConfigurationException ex)
+        {
+            Assert.assertEquals("initial_token was set but num_tokens is not!", ex.getMessage());
+        }
+    }
+
+    @Test
+    public void testApplyInitialTokensInitialTokensNotSetNumTokensSet()
+    {
+        Config config = DatabaseDescriptor.loadConfig();
+        config.num_tokens = 3;
+
+        DatabaseDescriptor.applyTokensConfig(config);
+
+        Assert.assertEquals(Integer.valueOf(3), config.num_tokens);
+        Assert.assertTrue(DatabaseDescriptor.tokensFromString(config.initial_token).isEmpty());
+    }
+
+    @Test
+    public void testApplyInitialTokensInitialTokensNotSetNumTokensNotSet()
+    {
+        Config config = DatabaseDescriptor.loadConfig();
+        DatabaseDescriptor.applyTokensConfig(config);
+
+        Assert.assertEquals(Integer.valueOf(1), config.num_tokens);
+        Assert.assertTrue(DatabaseDescriptor.tokensFromString(config.initial_token).isEmpty());
+    }
 }