diff --git a/CHANGES.txt b/CHANGES.txt index 83b8a618b9..69ccf55c5b 100644 --- a/CHANGES.txt +++ b/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) diff --git a/NEWS.txt b/NEWS.txt index de472c4506..cf33df9f75 100644 --- a/NEWS.txt +++ b/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 diff --git a/src/java/org/apache/cassandra/config/Config.java b/src/java/org/apache/cassandra/config/Config.java index 759a41a8ba..cde7d538ac 100644 --- a/src/java/org/apache/cassandra/config/Config.java +++ b/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 */ diff --git a/src/java/org/apache/cassandra/config/DatabaseDescriptor.java b/src/java/org/apache/cassandra/config/DatabaseDescriptor.java index 2be169a6ed..2eac706b62 100644 --- a/src/java/org/apache/cassandra/config/DatabaseDescriptor.java +++ b/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 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 diff --git a/test/unit/org/apache/cassandra/config/DatabaseDescriptorRefTest.java b/test/unit/org/apache/cassandra/config/DatabaseDescriptorRefTest.java index 181adbf73e..7dc84f6dbb 100644 --- a/test/unit/org/apache/cassandra/config/DatabaseDescriptorRefTest.java +++ b/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 diff --git a/test/unit/org/apache/cassandra/config/DatabaseDescriptorTest.java b/test/unit/org/apache/cassandra/config/DatabaseDescriptorTest.java index 2992a60ee5..19951038b8 100644 --- a/test/unit/org/apache/cassandra/config/DatabaseDescriptorTest.java +++ b/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()); + } }