andymg3 commented on code in PR #12305:
URL: https://github.com/apache/kafka/pull/12305#discussion_r933670311
##########
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:
@showuon @divijvaidya @hachikuji I took a little into adding a conditional
check. One thing that is tricky is the `isInternal` method
(https://github.com/apache/kafka/blob/trunk/clients/src/main/java/org/apache/kafka/common/internals/Topic.java#L59)
is used client side. For example, it’s used in TopicCommand.scala
(https://github.com/apache/kafka/blob/trunk/core/src/main/scala/kafka/admin/TopicCommand.scala#L427).
I don’t think it makes sense for the client to be in KRaft or ZK mode. During
an upgrade its possible there’s a mix or KRaft and ZK brokers, for example.
Furthermore, I can’t image we’d expect the client to provide metadata for
whether we’re in KRaft or ZK mode so I don’t know how we could know what mode
we’re even in.
Thus, I’m not sure how we can correctly handle sometimes including the the
__cluster_metadata topic and sometimes not. As I mentioned above, it seems
possible we will have this issue again in the future. Doesn’t seem unlikely new
internal topics will be created.
My feeling is we should include __cluster_metadata as an internal topic and
document it as a breaking change as such. Other solutions that start involving
the client feel overly complex. Open to thoughts and/or any other better ideas
folks might have though.
--
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
