krummas commented on code in PR #4614:
URL: https://github.com/apache/cassandra/pull/4614#discussion_r2822678756
##########
test/distributed/org/apache/cassandra/distributed/test/log/RegisterTest.java:
##########
@@ -187,8 +220,108 @@ public void replayLocallyFromV0Snapshot() throws Throwable
ClusterMetadata cm = new
MetadataSnapshots.SystemKeyspaceMetadataSnapshots().getSnapshot(ClusterMetadata.current().epoch);
cm.equals(ClusterMetadata.current());
});
+ }
+ }
+
+ /**
+ * Tests that registering a new node with a serialization version lower
than the cluster's
+ * commonSerializationVersion is rejected.
+ *
+ * Scenario:
+ * - Cluster has 1 node running at CURRENT_METADATA_VERSION (e.g., V8)
+ * - commonSerializationVersion = V8
+ * - A NEW node tries to register with V3
+ * - Should be REJECTED because V3 cannot read V8 metadata
+ */
+ @Test
+ public void testRegisterRejectsLowerSerializationVersion() throws Throwable
+ {
+ try (Cluster cluster = builder().withNodes(1).createWithoutStarting())
Review Comment:
It looks like these new tests could be unit tests instead, see
`PrepareJoinTest` for example
##########
src/java/org/apache/cassandra/config/CassandraRelevantProperties.java:
##########
@@ -677,6 +677,12 @@ public enum CassandraRelevantProperties
UCS_SURVIVAL_FACTOR("unified_compaction.survival_factor", "1"),
UCS_TARGET_SSTABLE_SIZE("unified_compaction.target_sstable_size", "1GiB"),
UDF_EXECUTOR_THREAD_KEEPALIVE_MS("cassandra.udf_executor_thread_keepalive_ms",
"30000"),
+ /**
+ * Skip the metadata serialization version check during node registration
and startup.
+ * WARNING: Only use this if you are certain the cluster metadata log
contains no entries
+ * that require a newer serialization version. Misuse can lead to
unreadable metadata.
+ */
+
UNSAFE_SKIP_SERIALIZATION_VERSION_CHECK("cassandra.unsafe.skip_serialization_version_check",
"false"),
Review Comment:
skip_metadata_serialization_version_check maybe?
##########
test/distributed/org/apache/cassandra/distributed/test/log/RegisterTest.java:
##########
@@ -108,23 +134,24 @@ public void serializationVersionCeilingTest() throws
Throwable
cluster.get(1).runOnInstance(() -> {
try
{
- // Register a ghost node with V0 to fake-force V0
serialization. In a real world cluster we will always be upgrading from a
smaller version.
+ // Unregister to make directory empty
Review Comment:
instead of unregistering we should use the new
`CassandraRevelantProperties.UNSAFE_SKIP_SERIALIZATION_VERSION_CHECK` to be
able to register the old-versioned instance
--
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]