frankgh commented on code in PR #4892:
URL: https://github.com/apache/cassandra/pull/4892#discussion_r3452899396
##########
test/unit/org/apache/cassandra/tcm/transformations/AlterSchemaMutationTrackingTest.java:
##########
@@ -372,4 +374,68 @@ private void
assertStatesEqual(MutationTrackingMigrationState expected, Mutation
assertEquals(expectedInfo, actualInfo);
}
}
+
+ /**
+ * The mutation journal can't be disabled for tracked replication, so
attempting to set durable_writes=false
+ * on tracked keyspaces needs to fail validation
+ */
+ @Test
+ public void testRejectDurableWritesFalseOnTrackedKeyspace()
+ {
+ // CREATE: tracked + durable_writes=false should be rejected
+ String createKs = nextKsName();
+ Throwable createFailure = expectFailure(() ->
+ schemaChange("CREATE KEYSPACE " + createKs +
+ " WITH replication = {'class': 'SimpleStrategy',
'replication_factor': '1'}" +
+ " AND replication_type = 'tracked'" +
+ " AND durable_writes = false")
+ );
+ assertTrue("Expected ConfigurationException root cause, got: " +
createFailure,
+ rootCause(createFailure) instanceof ConfigurationException);
Review Comment:
```suggestion
assertThatThrownBy(() ->
schemaChange("CREATE KEYSPACE " + createKs +
" WITH replication = {'class': 'SimpleStrategy',
'replication_factor': '1'}" +
" AND replication_type = 'tracked'" +
" AND durable_writes = false")
).hasRootCauseInstanceOf(ConfigurationException.class);
```
##########
test/unit/org/apache/cassandra/tcm/transformations/AlterSchemaMutationTrackingTest.java:
##########
@@ -372,4 +374,68 @@ private void
assertStatesEqual(MutationTrackingMigrationState expected, Mutation
assertEquals(expectedInfo, actualInfo);
}
}
+
+ /**
+ * The mutation journal can't be disabled for tracked replication, so
attempting to set durable_writes=false
+ * on tracked keyspaces needs to fail validation
+ */
+ @Test
+ public void testRejectDurableWritesFalseOnTrackedKeyspace()
+ {
+ // CREATE: tracked + durable_writes=false should be rejected
+ String createKs = nextKsName();
+ Throwable createFailure = expectFailure(() ->
+ schemaChange("CREATE KEYSPACE " + createKs +
+ " WITH replication = {'class': 'SimpleStrategy',
'replication_factor': '1'}" +
+ " AND replication_type = 'tracked'" +
+ " AND durable_writes = false")
+ );
+ assertTrue("Expected ConfigurationException root cause, got: " +
createFailure,
+ rootCause(createFailure) instanceof ConfigurationException);
+
+ // ALTER existing tracked keyspace to set durable_writes=false should
be rejected
+ String alterKs = nextKsName();
+ schemaChange("CREATE KEYSPACE " + alterKs +
+ " WITH replication = {'class': 'SimpleStrategy',
'replication_factor': '1'}" +
+ " AND replication_type = 'tracked'");
+ Throwable alterTrackedFailure = expectFailure(() ->
+ schemaChange("ALTER KEYSPACE " + alterKs + " WITH durable_writes =
false")
+ );
+ assertTrue("Expected ConfigurationException root cause, got: " +
alterTrackedFailure,
+ rootCause(alterTrackedFailure) instanceof
ConfigurationException);
+
+ // ALTER untracked keyspace with durable_writes=false to switch
replication_type=tracked should be rejected
+ String migratedKs = nextKsName();
+ schemaChange("CREATE KEYSPACE " + migratedKs +
+ " WITH replication = {'class': 'SimpleStrategy',
'replication_factor': '1'}" +
+ " AND replication_type = 'untracked'" +
+ " AND durable_writes = false");
+ Throwable alterToTrackedFailure = expectFailure(() ->
+ schemaChange("ALTER KEYSPACE " + migratedKs + " WITH
replication_type = 'tracked'")
+ );
+ assertTrue("Expected ConfigurationException root cause, got: " +
alterToTrackedFailure,
+ rootCause(alterToTrackedFailure) instanceof
ConfigurationException);
Review Comment:
```suggestion
assertThatThrownBy(() -> schemaChange("ALTER KEYSPACE " + migratedKs
+ " WITH replication_type =
'tracked'")).hasRootCauseInstanceOf(ConfigurationException.class);
```
##########
test/unit/org/apache/cassandra/tcm/transformations/AlterSchemaMutationTrackingTest.java:
##########
@@ -372,4 +374,68 @@ private void
assertStatesEqual(MutationTrackingMigrationState expected, Mutation
assertEquals(expectedInfo, actualInfo);
}
}
+
+ /**
+ * The mutation journal can't be disabled for tracked replication, so
attempting to set durable_writes=false
+ * on tracked keyspaces needs to fail validation
+ */
+ @Test
+ public void testRejectDurableWritesFalseOnTrackedKeyspace()
+ {
+ // CREATE: tracked + durable_writes=false should be rejected
+ String createKs = nextKsName();
+ Throwable createFailure = expectFailure(() ->
+ schemaChange("CREATE KEYSPACE " + createKs +
+ " WITH replication = {'class': 'SimpleStrategy',
'replication_factor': '1'}" +
+ " AND replication_type = 'tracked'" +
+ " AND durable_writes = false")
+ );
+ assertTrue("Expected ConfigurationException root cause, got: " +
createFailure,
+ rootCause(createFailure) instanceof ConfigurationException);
+
+ // ALTER existing tracked keyspace to set durable_writes=false should
be rejected
+ String alterKs = nextKsName();
+ schemaChange("CREATE KEYSPACE " + alterKs +
+ " WITH replication = {'class': 'SimpleStrategy',
'replication_factor': '1'}" +
+ " AND replication_type = 'tracked'");
+ Throwable alterTrackedFailure = expectFailure(() ->
+ schemaChange("ALTER KEYSPACE " + alterKs + " WITH durable_writes =
false")
+ );
+ assertTrue("Expected ConfigurationException root cause, got: " +
alterTrackedFailure,
+ rootCause(alterTrackedFailure) instanceof
ConfigurationException);
+
+ // ALTER untracked keyspace with durable_writes=false to switch
replication_type=tracked should be rejected
+ String migratedKs = nextKsName();
+ schemaChange("CREATE KEYSPACE " + migratedKs +
+ " WITH replication = {'class': 'SimpleStrategy',
'replication_factor': '1'}" +
+ " AND replication_type = 'untracked'" +
+ " AND durable_writes = false");
+ Throwable alterToTrackedFailure = expectFailure(() ->
+ schemaChange("ALTER KEYSPACE " + migratedKs + " WITH
replication_type = 'tracked'")
+ );
+ assertTrue("Expected ConfigurationException root cause, got: " +
alterToTrackedFailure,
+ rootCause(alterToTrackedFailure) instanceof
ConfigurationException);
+ }
+
+ private static Throwable expectFailure(Runnable r)
+ {
+ try
+ {
+ r.run();
+ }
+ catch (Throwable t)
+ {
+ return t;
+ }
+ fail("Expected exception but none was thrown");
+ return null;
+ }
+
+ private static Throwable rootCause(Throwable t)
+ {
+ Throwable cause = t;
+ while (cause.getCause() != null && cause.getCause() != cause)
+ cause = cause.getCause();
+ return cause;
+ }
Review Comment:
```suggestion
```
##########
test/unit/org/apache/cassandra/tcm/transformations/AlterSchemaMutationTrackingTest.java:
##########
Review Comment:
Can we instead use `import static
org.apache.cassandra.utils.AssertionUtils.assertThatThrownBy;` here?
##########
test/unit/org/apache/cassandra/tcm/transformations/AlterSchemaMutationTrackingTest.java:
##########
@@ -372,4 +374,68 @@ private void
assertStatesEqual(MutationTrackingMigrationState expected, Mutation
assertEquals(expectedInfo, actualInfo);
}
}
+
+ /**
+ * The mutation journal can't be disabled for tracked replication, so
attempting to set durable_writes=false
+ * on tracked keyspaces needs to fail validation
+ */
+ @Test
+ public void testRejectDurableWritesFalseOnTrackedKeyspace()
+ {
+ // CREATE: tracked + durable_writes=false should be rejected
+ String createKs = nextKsName();
+ Throwable createFailure = expectFailure(() ->
+ schemaChange("CREATE KEYSPACE " + createKs +
+ " WITH replication = {'class': 'SimpleStrategy',
'replication_factor': '1'}" +
+ " AND replication_type = 'tracked'" +
+ " AND durable_writes = false")
+ );
+ assertTrue("Expected ConfigurationException root cause, got: " +
createFailure,
+ rootCause(createFailure) instanceof ConfigurationException);
+
+ // ALTER existing tracked keyspace to set durable_writes=false should
be rejected
+ String alterKs = nextKsName();
+ schemaChange("CREATE KEYSPACE " + alterKs +
+ " WITH replication = {'class': 'SimpleStrategy',
'replication_factor': '1'}" +
+ " AND replication_type = 'tracked'");
+ Throwable alterTrackedFailure = expectFailure(() ->
+ schemaChange("ALTER KEYSPACE " + alterKs + " WITH durable_writes =
false")
+ );
+ assertTrue("Expected ConfigurationException root cause, got: " +
alterTrackedFailure,
+ rootCause(alterTrackedFailure) instanceof
ConfigurationException);
Review Comment:
```suggestion
assertThatThrownBy(() -> schemaChange("ALTER KEYSPACE " + alterKs +
" WITH durable_writes =
false")).hasRootCauseInstanceOf(ConfigurationException.class);
```
--
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]
