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]

Reply via email to