andymg3 commented on code in PR #12305:
URL: https://github.com/apache/kafka/pull/12305#discussion_r913275951


##########
clients/src/main/java/org/apache/kafka/common/internals/Topic.java:
##########
@@ -33,7 +33,7 @@ public class Topic {
     public static final String LEGAL_CHARS = "[a-zA-Z0-9._-]";
 
     private static final Set<String> INTERNAL_TOPICS = 
Collections.unmodifiableSet(
-            Utils.mkSet(GROUP_METADATA_TOPIC_NAME, 
TRANSACTION_STATE_TOPIC_NAME));
+            Utils.mkSet(GROUP_METADATA_TOPIC_NAME, 
TRANSACTION_STATE_TOPIC_NAME, METADATA_TOPIC_NAME));

Review Comment:
   Apologies for the late reply @divijvaidya. I think it's a good point about 
the compatibility issue. If a customer is relying on `isInternal` and they have 
a topic `__cluster_metadata` on an existing ZK cluster we do break them. We 
could wrap the code in a conditional check I think. @hachikuji thoughts? At 
first I thought this sounded a bit overkill but ultimately we dont know if 
folks are relying on this behavior and how critical it is to their 
applications. For upgrading from ZK to KRaft it seems like we'd have to handle 
the topic already existing somehow. But that issue is a bit separate from what 
@divijvaidya is concerned about (please correct me if I'm wrong) I think.
   
   This makes me wonder how we would handle other topics that we want to mark 
as internal in the future. It feels like maybe Kafka could/should reserve topic 
names that start with two underscores? Obviously this could also break 
applications, and would have to be thought through a lot and done carefully. 



-- 
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: jira-unsubscr...@kafka.apache.org

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

Reply via email to