[PR] MINOR: Relax conditions to reduce rebalances with the classic consumer [kafka]

["ezhou413 (via GitHub)" <gi...@apache.org>]

ezhou413 opened a new pull request, #22705:
URL: https://github.com/apache/kafka/pull/22705

   Delete this text and replace it with a detailed description of your change. The 
   PR title and body will become the squashed commit message.
   
   If you would like to tag individuals, add some commentary, upload images, or
   include other supplemental information that should not be part of the eventual
   commit message, please use a separate comment.
   
   If applicable, please include a summary of the testing strategy (including 
   rationale) for the proposed change. Unit and/or integration tests are expected
   for any behavior change and system tests should be considered for larger
   changes.
   


-- 
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-unsubscribe@kafka.apache.org

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

Re: [PR] MINOR: Relax conditions to reduce rebalances with the classic consumer [kafka]

["lianetm (via GitHub)" <gi...@apache.org>]

lianetm commented on code in PR #22705:
URL: https://github.com/apache/kafka/pull/22705#discussion_r3529692330


##########
clients/src/main/java/org/apache/kafka/clients/consumer/internals/ConsumerCoordinator.java:
##########
@@ -1656,36 +1677,44 @@ private ConsumerCoordinatorMetrics(MetricsLedger metrics, String metricGrpPrefix
     }
 
     private static class PartitionRackInfo {
-        private final Set<String> racks;
+        private final Set<String> knownRacks;
+        private final boolean hasUnknownReplica;

Review Comment:
   is a boolean enough or do we need to know the nodes that are down? E.g, what about there is a rack change (need to rebalance), at the same time that a replica is down? 
   
   Seems safer to be explicit about what we will tolerate and not rebalance. We have "known replicas" (with their racks, like the line above but wonder if we need to track a Map<replicaId, rack> instead), and "unknown replicas" (replicaIds that are down, have no host/rack). Then we can only safely tolerate a rack missing in one of them but present in the other for the same replica ID (that would be exactly the condition we want and nothing more: a replica X that either has the same racks it had before, or it's down). Makes sense? 



-- 
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-unsubscribe@kafka.apache.org

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

Re: [PR] MINOR: Relax conditions to reduce rebalances with the classic consumer [kafka]

["ezhou413 (via GitHub)" <gi...@apache.org>]

ezhou413 commented on PR #22705:
URL: https://github.com/apache/kafka/pull/22705#issuecomment-4917160952

   Fixed in #22768 


-- 
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-unsubscribe@kafka.apache.org

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

Re: [PR] MINOR: Relax conditions to reduce rebalances with the classic consumer [kafka]

["ezhou413 (via GitHub)" <gi...@apache.org>]

ezhou413 closed pull request #22705: MINOR: Relax conditions to reduce rebalances with the classic consumer
URL: https://github.com/apache/kafka/pull/22705


-- 
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-unsubscribe@kafka.apache.org

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