aparna0522 commented on code in PR #4614:
URL: https://github.com/apache/cassandra/pull/4614#discussion_r2813736793
##########
src/java/org/apache/cassandra/tcm/transformations/Startup.java:
##########
@@ -68,6 +68,19 @@ public Kind kind()
@Override
public Result execute(ClusterMetadata prev)
{
+ // Prevent downgrade to a version that cannot read cluster metadata.
+ // This protects against restarting a node with an older binary.
+ Version clusterVersion = prev.directory.commonSerializationVersion;
+ Version newNodeVersion = nodeVersion.serializationVersion();
+ if (newNodeVersion.isBefore(clusterVersion))
Review Comment:
Thanks for the review. I considered extracting this validation into
NodeVersion but I think I might not go with it for the following few reasons:
1. Separation of concerns: This check compares
nodeVersion.serializationVersion() against
prev.directory.commonSerializationVersion. The cluster's common version is
external context that NodeVersion shouldn't own since it's a cluster-level
property derived from the directory, not aa single node's version property.
2. Context-specific semantics: The checks serve different purposes in each
transformation. Register includes a guard that skips validation for the first
node in a new cluster (empty directory), whereas Startup performs the check
unconditionally since a restarting node always has an existing cluster to
compare against.
3. Avoiding inappropriate dependencies: NodeVersion is like a data class
responsible for holding version information and providing serialization. Adding
this validation would require NodeVersion to accept ClusterMetadata or
Directory as a parameter and return transformation-specific types like
Rejected. This would couple a low-level data model to higher-level coordination
abstractions. It could also inadveertently cause circular dependency
risks, since Directory already contains Map<NodeId, NodeVersion>.
If you have any other thoughts I am happy to discuss.
--
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]