[GitHub] [kafka] rondagostino commented on a change in pull request #10357: MINOR: KIP-500 Cluster ID is a String

[GitBox <gi...@apache.org>]

rondagostino commented on a change in pull request #10357:
URL: https://github.com/apache/kafka/pull/10357#discussion_r597324486



##########
File path: core/src/main/scala/kafka/tools/StorageTool.scala
##########
@@ -198,7 +198,7 @@ object StorageTool extends Logging {
         s"does not appear to be a valid UUID: ${e.getMessage}")
     }
     require(config.nodeId >= 0, s"The node.id must be set to a non-negative integer.")
-    new MetaProperties(effectiveClusterId, config.nodeId)
+    new MetaProperties(effectiveClusterId.toString, config.nodeId)

Review comment:
       > Should we also remove the [StorageTool] logic above which requires that the clusterId parses as a UUID 
   
   Basically, this is asking if `StorageTool` should support accepting non-UUIDs via its `--cluster-id` argument.  One purpose of the tool is to minimize the chance that a broker could use data from the wrong volume (i.e. data from another cluster).  Generating a random UUID via the `--random-uuid` parameter encourages using a globally unique value for every cluster and is consistent with the behavior today with ZooKeeper, whereas allowing a non-UUID here would increase the chance that someone could reuse a Cluster ID value across clusters and short-circuit the risk mitigation that this tool provides.
   
   Merging this PR now avoids an uncomfortable "rename" in `BrokerRegistrationRequest` (or a breaking change) after 2.8 is released; letting `StorageTool` accept non-UUIDs can be done at any time with no impact.  Maybe we keep this PR about removing the constraint, which also simply cannot exist -- otherwise we can't upgrade clusters that don't use a UUID -- and leave this question about the `--cluster-id` parameter for a broader discussion later?  




-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org