azotcsit commented on a change in pull request #1229:
URL: https://github.com/apache/cassandra/pull/1229#discussion_r717053000
##########
File path:
src/java/org/apache/cassandra/tools/nodetool/GetDefaultKeyspaceRF.java
##########
@@ -0,0 +1,32 @@
+/*
Review comment:
I think it makes sense to write unit tests for the new nodetool commands
(similar to `GossipInfoTest`). Even though the logic is trivial, having tests
helps to keep code coverage on a high level and ensures nothing is broken
occasionally.
##########
File path: src/java/org/apache/cassandra/locator/SimpleStrategy.java
##########
@@ -123,4 +124,13 @@ public void maybeWarnOnOptions()
{
return Collections.singleton(REPLICATION_FACTOR);
}
+
+ protected static void prepareOptions(Map<String, String> options,
Map<String, String> previousOptions)
+ {
+ // When altering from NTS to SS, previousOptions could have multiple
different RFs for different data centers - so we
+ // will instead default to DefaultRF configuration if RF is not
mentioned with the alter statement
+ String rf = previousOptions.containsKey(REPLICATION_FACTOR) ?
previousOptions.get(REPLICATION_FACTOR)
+ :
Integer.toString(DatabaseDescriptor.getDefaultKeyspaceRF());
+ options.putIfAbsent("replication_factor", rf);
Review comment:
I think we can use `REPLICATION_FACTOR` constant here as well.
##########
File path: test/unit/org/apache/cassandra/config/DatabaseDescriptorTest.java
##########
@@ -590,4 +590,23 @@ public void
testApplyTokensConfigInitialTokensOneNumTokensNotSet()
Assert.assertEquals(Integer.valueOf(1), config.num_tokens);
Assert.assertEquals(1,
DatabaseDescriptor.tokensFromString(config.initial_token).size());
}
+
+ @Test (expected = ConfigurationException.class)
+ public void testInvalidSub1DefaultRFs() throws ConfigurationException
+ {
+ DatabaseDescriptor.setDefaultKeyspaceRF(0);
+ }
+
+ @Test (expected = ConfigurationException.class)
+ public void testInvalidSub0MinimumRFs() throws ConfigurationException
+ {
+ DatabaseDescriptor.setMinimumKeyspaceRF(-1);
+ }
+
+ @Test (expected = ConfigurationException.class)
+ public void testDefaultRfLessThanMinRF()
Review comment:
This one also does not seem to be covered:
```
if (conf.default_keyspace_rf < conf.minimum_keyspace_rf)
```
##########
File path: test/unit/org/apache/cassandra/config/DatabaseDescriptorTest.java
##########
@@ -590,4 +590,23 @@ public void
testApplyTokensConfigInitialTokensOneNumTokensNotSet()
Assert.assertEquals(Integer.valueOf(1), config.num_tokens);
Assert.assertEquals(1,
DatabaseDescriptor.tokensFromString(config.initial_token).size());
}
+
+ @Test (expected = ConfigurationException.class)
+ public void testInvalidSub1DefaultRFs() throws ConfigurationException
+ {
+ DatabaseDescriptor.setDefaultKeyspaceRF(0);
+ }
+
+ @Test (expected = ConfigurationException.class)
+ public void testInvalidSub0MinimumRFs() throws ConfigurationException
+ {
+ DatabaseDescriptor.setMinimumKeyspaceRF(-1);
+ }
+
+ @Test (expected = ConfigurationException.class)
+ public void testDefaultRfLessThanMinRF()
Review comment:
I think it makes sense to add one more test where we set default at
first and then minimum and it triggers the exception. I think this condition is
not covered at the moment.
##########
File path:
test/unit/org/apache/cassandra/cql3/validation/operations/AlterTest.java
##########
@@ -340,27 +345,128 @@ public void testCreateSimpleAlterNTSDefaults() throws
Throwable
row(ks1, true, map("class",
"org.apache.cassandra.locator.NetworkTopologyStrategy", DATA_CENTER, "2",
DATA_CENTER_REMOTE, "2")));
}
- /**
- * Test {@link ConfigurationException} thrown on alter keyspace to no DC
option in replication configuration.
- */
@Test
- public void testAlterKeyspaceWithNoOptionThrowsConfigurationException()
throws Throwable
+ public void testDefaultRF() throws Throwable
+ {
+ TokenMetadata metadata = StorageService.instance.getTokenMetadata();
+ metadata.clearUnsafe();
+ InetAddressAndPort local = FBUtilities.getBroadcastAddressAndPort();
+ InetAddressAndPort remote = InetAddressAndPort.getByName("127.0.0.4");
+ metadata.updateHostId(UUID.randomUUID(), local);
+ metadata.updateNormalToken(new
OrderPreservingPartitioner.StringToken("A"), local);
+ metadata.updateHostId(UUID.randomUUID(), remote);
+ metadata.updateNormalToken(new
OrderPreservingPartitioner.StringToken("B"), remote);
+
+ DatabaseDescriptor.setDefaultKeyspaceRF(3);
+
+ //ensure default rf is being taken into account during creation, and
user can choose to override the default
+ String ks1 = createKeyspace("CREATE KEYSPACE %s WITH replication={
'class' : 'SimpleStrategy' }");
+ String ks2 = createKeyspace("CREATE KEYSPACE %s WITH replication={
'class' : 'SimpleStrategy', 'replication_factor' : 2 }");
+ String ks3 = createKeyspace("CREATE KEYSPACE %s WITH replication={
'class' : 'NetworkTopologyStrategy' }");
+ String ks4 = createKeyspace("CREATE KEYSPACE %s WITH replication={
'class' : 'NetworkTopologyStrategy', 'replication_factor' : 2 }");
+
+ assertRowsIgnoringOrderAndExtra(execute("SELECT keyspace_name,
durable_writes, replication FROM system_schema.keyspaces"),
+ row(ks1, true,
map("class","org.apache.cassandra.locator.SimpleStrategy","replication_factor",
Integer.toString(DatabaseDescriptor.getDefaultKeyspaceRF()))),
+ row(ks2, true,
map("class","org.apache.cassandra.locator.SimpleStrategy","replication_factor",
"2")),
+ row(ks3, true, map("class",
"org.apache.cassandra.locator.NetworkTopologyStrategy", DATA_CENTER,
+
Integer.toString(DatabaseDescriptor.getDefaultKeyspaceRF()),
DATA_CENTER_REMOTE,
Integer.toString(DatabaseDescriptor.getDefaultKeyspaceRF()))),
+ row(ks4, true, map("class",
"org.apache.cassandra.locator.NetworkTopologyStrategy", DATA_CENTER, "2",
DATA_CENTER_REMOTE, "2")));
+
+
+
+ //ensure alter keyspace does not default to default rf unless altering
from NTS to SS
+ //no change alter
+ schemaChange("ALTER KEYSPACE " + ks4 + " WITH durable_writes=true");
+ assertRowsIgnoringOrderAndExtra(execute("SELECT keyspace_name,
durable_writes, replication FROM system_schema.keyspaces"),
+ row(ks4, true, map("class",
"org.apache.cassandra.locator.NetworkTopologyStrategy", DATA_CENTER, "2",
DATA_CENTER_REMOTE, "2")));
+ schemaChange("ALTER KEYSPACE " + ks4 + " WITH replication={ 'class' :
'NetworkTopologyStrategy' }");
+ assertRowsIgnoringOrderAndExtra(execute("SELECT keyspace_name,
durable_writes, replication FROM system_schema.keyspaces"),
+ row(ks4, true, map("class",
"org.apache.cassandra.locator.NetworkTopologyStrategy", DATA_CENTER, "2",
DATA_CENTER_REMOTE, "2")));
+
+ // change from SS to NTS
+ // without specifying RF
+ schemaChange("ALTER KEYSPACE " + ks2 + " WITH replication={ 'class' :
'NetworkTopologyStrategy' } AND durable_writes=true");
+ // verify that RF of SS is retained
+ assertRowsIgnoringOrderAndExtra(execute("SELECT keyspace_name,
durable_writes, replication FROM system_schema.keyspaces"),
+ row(ks2, true,
map("class","org.apache.cassandra.locator.NetworkTopologyStrategy",
DATA_CENTER, "2", DATA_CENTER_REMOTE, "2")));
+ // with specifying RF
+ schemaChange("ALTER KEYSPACE " + ks1 + " WITH replication={ 'class' :
'NetworkTopologyStrategy', 'replication_factor': '1' } AND
durable_writes=true");
+ // verify that explicitly mentioned RF is taken into account
+ assertRowsIgnoringOrderAndExtra(execute("SELECT keyspace_name,
durable_writes, replication FROM system_schema.keyspaces"),
+ row(ks1, true,
map("class","org.apache.cassandra.locator.NetworkTopologyStrategy",
DATA_CENTER, "1", DATA_CENTER_REMOTE, "1")));
+
+ // change from NTS to SS
+ // without specifying RF
+ schemaChange("ALTER KEYSPACE " + ks4 + " WITH replication={ 'class' :
'SimpleStrategy' } AND durable_writes=true");
+ // verify that default RF is taken into account
+ assertRowsIgnoringOrderAndExtra(execute("SELECT keyspace_name,
durable_writes, replication FROM system_schema.keyspaces"),
+ row(ks4, true,
map("class","org.apache.cassandra.locator.SimpleStrategy","replication_factor",
Integer.toString(DatabaseDescriptor.getDefaultKeyspaceRF()))));
+ // with specifying RF
+ schemaChange("ALTER KEYSPACE " + ks3 + " WITH replication={ 'class' :
'SimpleStrategy', 'replication_factor' : '1' } AND durable_writes=true");
+ // verify that explicitly mentioned RF is taken into account
+ assertRowsIgnoringOrderAndExtra(execute("SELECT keyspace_name,
durable_writes, replication FROM system_schema.keyspaces"),
+ row(ks3, true,
map("class","org.apache.cassandra.locator.SimpleStrategy","replication_factor",
"1")));
+
+
+ // verify updated default does not effect existing keyspaces
+ // create keyspaces
+ String ks5 = createKeyspace("CREATE KEYSPACE %s WITH replication={
'class' : 'SimpleStrategy' }");
+ String ks6 = createKeyspace("CREATE KEYSPACE %s WITH replication={
'class' : 'NetworkTopologyStrategy' }");
+ String oldRF =
Integer.toString(DatabaseDescriptor.getDefaultKeyspaceRF());
+ // change default
+ DatabaseDescriptor.setDefaultKeyspaceRF(2);
+ // verify RF of existing keyspaces
+ assertRowsIgnoringOrderAndExtra(execute("SELECT keyspace_name,
durable_writes, replication FROM system_schema.keyspaces"),
+ row(ks5, true,
map("class","org.apache.cassandra.locator.SimpleStrategy","replication_factor",
oldRF)));
+ assertRowsIgnoringOrderAndExtra(execute("SELECT keyspace_name,
durable_writes, replication FROM system_schema.keyspaces"),
+ row(ks6, true, map("class",
"org.apache.cassandra.locator.NetworkTopologyStrategy",
+ DATA_CENTER, oldRF,
DATA_CENTER_REMOTE, oldRF)));
+
+ //clean up config change
+ DatabaseDescriptor.setDefaultKeyspaceRF(1);
+
+ //clean up keyspaces
+ execute(String.format("DROP KEYSPACE IF EXISTS %s", ks1));
+ execute(String.format("DROP KEYSPACE IF EXISTS %s", ks2));
+ execute(String.format("DROP KEYSPACE IF EXISTS %s", ks3));
+ execute(String.format("DROP KEYSPACE IF EXISTS %s", ks4));
+ execute(String.format("DROP KEYSPACE IF EXISTS %s", ks5));
+ execute(String.format("DROP KEYSPACE IF EXISTS %s", ks6));
+ }
+
+ @Test
+ public void testMinimumRF() throws Throwable
{
- // Create keyspaces
- execute("CREATE KEYSPACE testABC WITH replication={ 'class' :
'NetworkTopologyStrategy', '" + DATA_CENTER + "' : 3 }");
- execute("CREATE KEYSPACE testXYZ WITH replication={ 'class' :
'SimpleStrategy', 'replication_factor' : 3 }");
+ DatabaseDescriptor.setDefaultKeyspaceRF(3);
+ DatabaseDescriptor.setMinimumKeyspaceRF(2);
+
+ assertAlterTableThrowsException(ConfigurationException.class,
+ String.format("Replication factor
cannot be less than minimum_keyspace_rf (%s), found %s",
DatabaseDescriptor.getMinimumKeyspaceRF(), "1"),
+ "CREATE KEYSPACE ks1 WITH
replication={ 'class' : 'SimpleStrategy', 'replication_factor' : 1 }");
+
+ assertAlterTableThrowsException(ConfigurationException.class,
+ String.format("Replication factor
cannot be less than minimum_keyspace_rf (%s), found %s",
DatabaseDescriptor.getMinimumKeyspaceRF(), "1"),
+ "CREATE KEYSPACE ks2 WITH
replication={ 'class' : 'NetworkTopologyStrategy', 'replication_factor' : 1 }");
+
+ String ks1 = createKeyspace("CREATE KEYSPACE %s WITH replication={
'class' : 'SimpleStrategy' }");
+ String ks2 = createKeyspace("CREATE KEYSPACE %s WITH replication={
'class' : 'NetworkTopologyStrategy' }");
+
+ assertAlterTableThrowsException(ConfigurationException.class,
+ String.format("Replication factor
cannot be less than minimum_keyspace_rf (%s), found %s",
DatabaseDescriptor.getMinimumKeyspaceRF(), "1"),
+ String.format("ALTER KEYSPACE %s WITH
replication={ 'class' : 'SimpleStrategy', 'replication_factor' : 1 }", ks1));
+ assertAlterTableThrowsException(ConfigurationException.class,
+ String.format("Replication factor
cannot be less than minimum_keyspace_rf (%s), found %s",
DatabaseDescriptor.getMinimumKeyspaceRF(), "1"),
+ String.format("ALTER KEYSPACE %s WITH
replication={ 'class' : 'NetworkTopologyStrategy', '" + DATA_CENTER + "' : '1'
}", ks2));
- // Try to alter the created keyspace without any option
- assertInvalidThrow(ConfigurationException.class, "ALTER KEYSPACE
testABC WITH replication={ 'class' : 'NetworkTopologyStrategy' }");
- assertInvalidThrow(ConfigurationException.class, "ALTER KEYSPACE
testXYZ WITH replication={ 'class' : 'SimpleStrategy' }");
+ createKeyspace("CREATE KEYSPACE %s WITH replication={ 'class' :
'NetworkTopologyStrategy', '" + DATA_CENTER + "' : 2}");
Review comment:
I'm not sure it is used, probably it needs to be removed. I might be
missing smth.
##########
File path: src/java/org/apache/cassandra/locator/SimpleStrategy.java
##########
@@ -123,4 +124,13 @@ public void maybeWarnOnOptions()
{
return Collections.singleton(REPLICATION_FACTOR);
}
+
+ protected static void prepareOptions(Map<String, String> options,
Map<String, String> previousOptions)
+ {
+ // When altering from NTS to SS, previousOptions could have multiple
different RFs for different data centers - so we
+ // will instead default to DefaultRF configuration if RF is not
mentioned with the alter statement
+ String rf = previousOptions.containsKey(REPLICATION_FACTOR) ?
previousOptions.get(REPLICATION_FACTOR)
+ :
Integer.toString(DatabaseDescriptor.getDefaultKeyspaceRF());
Review comment:
nit: seems like there are two extra spaces between ":"
##########
File path:
test/unit/org/apache/cassandra/cql3/validation/operations/AlterTest.java
##########
@@ -340,27 +345,128 @@ public void testCreateSimpleAlterNTSDefaults() throws
Throwable
row(ks1, true, map("class",
"org.apache.cassandra.locator.NetworkTopologyStrategy", DATA_CENTER, "2",
DATA_CENTER_REMOTE, "2")));
}
- /**
- * Test {@link ConfigurationException} thrown on alter keyspace to no DC
option in replication configuration.
- */
@Test
- public void testAlterKeyspaceWithNoOptionThrowsConfigurationException()
throws Throwable
+ public void testDefaultRF() throws Throwable
+ {
+ TokenMetadata metadata = StorageService.instance.getTokenMetadata();
+ metadata.clearUnsafe();
+ InetAddressAndPort local = FBUtilities.getBroadcastAddressAndPort();
+ InetAddressAndPort remote = InetAddressAndPort.getByName("127.0.0.4");
+ metadata.updateHostId(UUID.randomUUID(), local);
+ metadata.updateNormalToken(new
OrderPreservingPartitioner.StringToken("A"), local);
+ metadata.updateHostId(UUID.randomUUID(), remote);
+ metadata.updateNormalToken(new
OrderPreservingPartitioner.StringToken("B"), remote);
+
+ DatabaseDescriptor.setDefaultKeyspaceRF(3);
+
+ //ensure default rf is being taken into account during creation, and
user can choose to override the default
+ String ks1 = createKeyspace("CREATE KEYSPACE %s WITH replication={
'class' : 'SimpleStrategy' }");
+ String ks2 = createKeyspace("CREATE KEYSPACE %s WITH replication={
'class' : 'SimpleStrategy', 'replication_factor' : 2 }");
+ String ks3 = createKeyspace("CREATE KEYSPACE %s WITH replication={
'class' : 'NetworkTopologyStrategy' }");
+ String ks4 = createKeyspace("CREATE KEYSPACE %s WITH replication={
'class' : 'NetworkTopologyStrategy', 'replication_factor' : 2 }");
+
+ assertRowsIgnoringOrderAndExtra(execute("SELECT keyspace_name,
durable_writes, replication FROM system_schema.keyspaces"),
+ row(ks1, true,
map("class","org.apache.cassandra.locator.SimpleStrategy","replication_factor",
Integer.toString(DatabaseDescriptor.getDefaultKeyspaceRF()))),
+ row(ks2, true,
map("class","org.apache.cassandra.locator.SimpleStrategy","replication_factor",
"2")),
+ row(ks3, true, map("class",
"org.apache.cassandra.locator.NetworkTopologyStrategy", DATA_CENTER,
+
Integer.toString(DatabaseDescriptor.getDefaultKeyspaceRF()),
DATA_CENTER_REMOTE,
Integer.toString(DatabaseDescriptor.getDefaultKeyspaceRF()))),
+ row(ks4, true, map("class",
"org.apache.cassandra.locator.NetworkTopologyStrategy", DATA_CENTER, "2",
DATA_CENTER_REMOTE, "2")));
+
+
+
+ //ensure alter keyspace does not default to default rf unless altering
from NTS to SS
+ //no change alter
+ schemaChange("ALTER KEYSPACE " + ks4 + " WITH durable_writes=true");
+ assertRowsIgnoringOrderAndExtra(execute("SELECT keyspace_name,
durable_writes, replication FROM system_schema.keyspaces"),
+ row(ks4, true, map("class",
"org.apache.cassandra.locator.NetworkTopologyStrategy", DATA_CENTER, "2",
DATA_CENTER_REMOTE, "2")));
+ schemaChange("ALTER KEYSPACE " + ks4 + " WITH replication={ 'class' :
'NetworkTopologyStrategy' }");
+ assertRowsIgnoringOrderAndExtra(execute("SELECT keyspace_name,
durable_writes, replication FROM system_schema.keyspaces"),
+ row(ks4, true, map("class",
"org.apache.cassandra.locator.NetworkTopologyStrategy", DATA_CENTER, "2",
DATA_CENTER_REMOTE, "2")));
+
+ // change from SS to NTS
+ // without specifying RF
+ schemaChange("ALTER KEYSPACE " + ks2 + " WITH replication={ 'class' :
'NetworkTopologyStrategy' } AND durable_writes=true");
+ // verify that RF of SS is retained
+ assertRowsIgnoringOrderAndExtra(execute("SELECT keyspace_name,
durable_writes, replication FROM system_schema.keyspaces"),
+ row(ks2, true,
map("class","org.apache.cassandra.locator.NetworkTopologyStrategy",
DATA_CENTER, "2", DATA_CENTER_REMOTE, "2")));
+ // with specifying RF
+ schemaChange("ALTER KEYSPACE " + ks1 + " WITH replication={ 'class' :
'NetworkTopologyStrategy', 'replication_factor': '1' } AND
durable_writes=true");
+ // verify that explicitly mentioned RF is taken into account
+ assertRowsIgnoringOrderAndExtra(execute("SELECT keyspace_name,
durable_writes, replication FROM system_schema.keyspaces"),
+ row(ks1, true,
map("class","org.apache.cassandra.locator.NetworkTopologyStrategy",
DATA_CENTER, "1", DATA_CENTER_REMOTE, "1")));
+
+ // change from NTS to SS
+ // without specifying RF
+ schemaChange("ALTER KEYSPACE " + ks4 + " WITH replication={ 'class' :
'SimpleStrategy' } AND durable_writes=true");
+ // verify that default RF is taken into account
+ assertRowsIgnoringOrderAndExtra(execute("SELECT keyspace_name,
durable_writes, replication FROM system_schema.keyspaces"),
+ row(ks4, true,
map("class","org.apache.cassandra.locator.SimpleStrategy","replication_factor",
Integer.toString(DatabaseDescriptor.getDefaultKeyspaceRF()))));
+ // with specifying RF
+ schemaChange("ALTER KEYSPACE " + ks3 + " WITH replication={ 'class' :
'SimpleStrategy', 'replication_factor' : '1' } AND durable_writes=true");
+ // verify that explicitly mentioned RF is taken into account
+ assertRowsIgnoringOrderAndExtra(execute("SELECT keyspace_name,
durable_writes, replication FROM system_schema.keyspaces"),
+ row(ks3, true,
map("class","org.apache.cassandra.locator.SimpleStrategy","replication_factor",
"1")));
+
+
+ // verify updated default does not effect existing keyspaces
+ // create keyspaces
+ String ks5 = createKeyspace("CREATE KEYSPACE %s WITH replication={
'class' : 'SimpleStrategy' }");
+ String ks6 = createKeyspace("CREATE KEYSPACE %s WITH replication={
'class' : 'NetworkTopologyStrategy' }");
+ String oldRF =
Integer.toString(DatabaseDescriptor.getDefaultKeyspaceRF());
+ // change default
+ DatabaseDescriptor.setDefaultKeyspaceRF(2);
+ // verify RF of existing keyspaces
+ assertRowsIgnoringOrderAndExtra(execute("SELECT keyspace_name,
durable_writes, replication FROM system_schema.keyspaces"),
+ row(ks5, true,
map("class","org.apache.cassandra.locator.SimpleStrategy","replication_factor",
oldRF)));
+ assertRowsIgnoringOrderAndExtra(execute("SELECT keyspace_name,
durable_writes, replication FROM system_schema.keyspaces"),
+ row(ks6, true, map("class",
"org.apache.cassandra.locator.NetworkTopologyStrategy",
+ DATA_CENTER, oldRF,
DATA_CENTER_REMOTE, oldRF)));
+
+ //clean up config change
+ DatabaseDescriptor.setDefaultKeyspaceRF(1);
+
+ //clean up keyspaces
+ execute(String.format("DROP KEYSPACE IF EXISTS %s", ks1));
+ execute(String.format("DROP KEYSPACE IF EXISTS %s", ks2));
+ execute(String.format("DROP KEYSPACE IF EXISTS %s", ks3));
+ execute(String.format("DROP KEYSPACE IF EXISTS %s", ks4));
+ execute(String.format("DROP KEYSPACE IF EXISTS %s", ks5));
+ execute(String.format("DROP KEYSPACE IF EXISTS %s", ks6));
+ }
+
+ @Test
+ public void testMinimumRF() throws Throwable
{
- // Create keyspaces
- execute("CREATE KEYSPACE testABC WITH replication={ 'class' :
'NetworkTopologyStrategy', '" + DATA_CENTER + "' : 3 }");
- execute("CREATE KEYSPACE testXYZ WITH replication={ 'class' :
'SimpleStrategy', 'replication_factor' : 3 }");
+ DatabaseDescriptor.setDefaultKeyspaceRF(3);
+ DatabaseDescriptor.setMinimumKeyspaceRF(2);
+
+ assertAlterTableThrowsException(ConfigurationException.class,
Review comment:
Does it make sense to move first two asserts to `CreateTest` since they
are about testing `CREATE` statement?
##########
File path:
test/unit/org/apache/cassandra/cql3/validation/operations/AlterTest.java
##########
@@ -340,27 +345,128 @@ public void testCreateSimpleAlterNTSDefaults() throws
Throwable
row(ks1, true, map("class",
"org.apache.cassandra.locator.NetworkTopologyStrategy", DATA_CENTER, "2",
DATA_CENTER_REMOTE, "2")));
}
- /**
- * Test {@link ConfigurationException} thrown on alter keyspace to no DC
option in replication configuration.
- */
@Test
- public void testAlterKeyspaceWithNoOptionThrowsConfigurationException()
throws Throwable
+ public void testDefaultRF() throws Throwable
+ {
+ TokenMetadata metadata = StorageService.instance.getTokenMetadata();
+ metadata.clearUnsafe();
+ InetAddressAndPort local = FBUtilities.getBroadcastAddressAndPort();
+ InetAddressAndPort remote = InetAddressAndPort.getByName("127.0.0.4");
+ metadata.updateHostId(UUID.randomUUID(), local);
+ metadata.updateNormalToken(new
OrderPreservingPartitioner.StringToken("A"), local);
+ metadata.updateHostId(UUID.randomUUID(), remote);
+ metadata.updateNormalToken(new
OrderPreservingPartitioner.StringToken("B"), remote);
+
+ DatabaseDescriptor.setDefaultKeyspaceRF(3);
+
+ //ensure default rf is being taken into account during creation, and
user can choose to override the default
+ String ks1 = createKeyspace("CREATE KEYSPACE %s WITH replication={
'class' : 'SimpleStrategy' }");
+ String ks2 = createKeyspace("CREATE KEYSPACE %s WITH replication={
'class' : 'SimpleStrategy', 'replication_factor' : 2 }");
+ String ks3 = createKeyspace("CREATE KEYSPACE %s WITH replication={
'class' : 'NetworkTopologyStrategy' }");
+ String ks4 = createKeyspace("CREATE KEYSPACE %s WITH replication={
'class' : 'NetworkTopologyStrategy', 'replication_factor' : 2 }");
+
+ assertRowsIgnoringOrderAndExtra(execute("SELECT keyspace_name,
durable_writes, replication FROM system_schema.keyspaces"),
+ row(ks1, true,
map("class","org.apache.cassandra.locator.SimpleStrategy","replication_factor",
Integer.toString(DatabaseDescriptor.getDefaultKeyspaceRF()))),
+ row(ks2, true,
map("class","org.apache.cassandra.locator.SimpleStrategy","replication_factor",
"2")),
+ row(ks3, true, map("class",
"org.apache.cassandra.locator.NetworkTopologyStrategy", DATA_CENTER,
+
Integer.toString(DatabaseDescriptor.getDefaultKeyspaceRF()),
DATA_CENTER_REMOTE,
Integer.toString(DatabaseDescriptor.getDefaultKeyspaceRF()))),
+ row(ks4, true, map("class",
"org.apache.cassandra.locator.NetworkTopologyStrategy", DATA_CENTER, "2",
DATA_CENTER_REMOTE, "2")));
+
+
Review comment:
nit: too many new lines
##########
File path: src/java/org/apache/cassandra/config/Config.java
##########
@@ -482,6 +482,11 @@
public volatile boolean diagnostic_events_enabled = false;
+ // Default and minimum keyspace replication factors allow validation of
newly created keyspaces
+ // and good defaults if no replication factor is provided by the user
+ public int default_keyspace_rf = 1;
Review comment:
Should this be volatile? I'm not sure, but I have a suspicion that all
fields supposed to be updated are marked `volatile`. I hope someone from
project veterans could confirm the assumption.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
