dajac commented on code in PR #17444:
URL: https://github.com/apache/kafka/pull/17444#discussion_r1815360708


##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/modern/ModernGroup.java:
##########
@@ -388,10 +386,25 @@ public Map<String, TopicMetadata> 
computeSubscriptionMetadata(
         subscribedTopicNames.forEach((topicName, count) -> {
             TopicImage topicImage = topicsImage.getTopic(topicName);
             if (topicImage != null) {
+                Map<Integer, Set<String>> partitionRacks = new HashMap<>();
+                topicImage.partitions().forEach((partition, 
partitionRegistration) -> {
+                    Set<String> racks = new HashSet<>();
+                    for (int replica : partitionRegistration.replicas) {
+                        Optional<String> rackOptional = 
clusterImage.broker(replica).rack();

Review Comment:
   @chia7712 It is actually a gap in KIP-848 to include 
https://cwiki.apache.org/confluence/display/KAFKA/KIP-881%3A+Rack-aware+Partition+Assignment+for+Kafka+Consumers.
   
   > On another note, moving a partition can trigger a rack change, correct? If 
so, could this cause issues for users who have deployed an auto-rebalance tool? 
It might lead to a series of rebalances.
   
   It should not cause issues because the assignment should only change if the 
consumers are not aligned with racks anymore. Otherwise, it should stay the 
same.



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

Reply via email to