FrankYang0529 commented on code in PR #17444:
URL: https://github.com/apache/kafka/pull/17444#discussion_r1810823291
##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/modern/SubscribedTopicDescriberImpl.java:
##########
@@ -72,7 +71,8 @@ public int numPartitions(Uuid topicId) {
*/
@Override
public Set<String> racksForPartition(Uuid topicId, int partition) {
- return Collections.emptySet();
+ TopicMetadata topic = this.topicMetadata.get(topicId);
+ return topic == null ? Set.of() :
topic.partitionRacks().getOrDefault(partition, Set.of());
Review Comment:
Hi @dajac, sorry, I'm not clear about this comment.
In current design, we always calculate `Map<String, TopicMetadata>` before
calling `upgradeTarget`. Even if group doesn't maintain it, we still have data
here.
https://github.com/apache/kafka/blob/8a3292faa826a54c99761c485d083bf421bb1392/group-coordinator/src/main/java/org/apache/kafka/coordinator/group/GroupMetadataManager.java#L1838-L1848
If we would like to use `MetadataImage` here, it looks like we only
calculate `Map<String, TopicMetadata>` for hash and record it. We don't use it
for any further function. For functionality, I think it also works. Just want
to check is that your thought? Thank you.
--
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]
