lianetm commented on code in PR #19814:
URL: https://github.com/apache/kafka/pull/19814#discussion_r2132875505
##########
clients/src/main/java/org/apache/kafka/clients/consumer/internals/ConsumerMetadata.java:
##########
@@ -66,14 +67,24 @@ public boolean allowAutoTopicCreation() {
return allowAutoTopicCreation;
}
+ /**
+ * Constructs a metadata request builder for fetching cluster metadata for
the topics the consumer needs.
+ * This will include:
+ * <ul>
+ * <li>topics the consumer is subscribed to using topic names (calls
to subscribe with topic name list or client-side regex)</li>
+ * <li>topics the consumer is subscribed to using topic IDs (calls to
subscribe with broker-side regex RE2J)</li>
+ * <li>topics involved in calls for fetching offsets (transient
topics)</li>
+ * </ul>
+ * Note that this will generate a request for all topics in the cluster
only when the consumer is subscribed to a client-side regex.
+ */
@Override
public synchronized MetadataRequest.Builder newMetadataRequestBuilder() {
- if (subscription.hasPatternSubscription() ||
subscription.hasRe2JPatternSubscription())
+ if (subscription.hasPatternSubscription())
return MetadataRequest.Builder.allTopics();
List<String> topics = new ArrayList<>();
topics.addAll(subscription.metadataTopics());
topics.addAll(transientTopics);
- return new MetadataRequest.Builder(topics, allowAutoTopicCreation);
+ return new MetadataRequest.Builder(topics,
subscription.assignedTopicIds(), allowAutoTopicCreation);
Review Comment:
good catch (mismatch between the javadoc and code because I did add the code
at some point but seems I ended up removing it unintentionally).
You're right that we could need metadata for assigned topic IDs and
transient topics (names), but since the metadata request handling on the broker
does not support that we have no choice but to request them all. Added the fix.
Since we're here, curious, would it make sense to review that broker-side
logic to process metadata requests and take both, names and IDs if present?
(this bit, that at the moment only considers topic names if no topic IDs
provided
https://github.com/apache/kafka/blob/c4a769bc8be54fe07e0ed5f05f00b48aa02a8404/core/src/main/scala/kafka/server/KafkaApis.scala#L899-L902)
--
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]
