showuon commented on code in PR #12066:
URL: https://github.com/apache/kafka/pull/12066#discussion_r854734845


##########
clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordAccumulator.java:
##########
@@ -559,13 +560,12 @@ private List<ProducerBatch> 
drainBatchesForOneNode(Cluster cluster, Node node, i
         int size = 0;
         List<PartitionInfo> parts = cluster.partitionsForNode(node.id());
         List<ProducerBatch> ready = new ArrayList<>();
-        /* to make starvation less likely this loop doesn't start at 0 */
+        /* to make starvation less likely each node has it's own drainIndex */
+        int drainIndex = getDrainIndex(node.idString());
         int start = drainIndex = drainIndex % parts.size();
         do {
             PartitionInfo part = parts.get(drainIndex);
             TopicPartition tp = new TopicPartition(part.topic(), 
part.partition());
-            this.drainIndex = (this.drainIndex + 1) % parts.size();
-
             // Only proceed if the partition has no in-flight batches.
             if (isMuted(tp))
                 continue;

Review Comment:
   Have a 2nd look, I found this change is not right. If the tp is muted or 
other reason, it `continued`, and we'll have a infinite loop because the 
drainIdex won't get incremented, right? Please fix it and add a test for this 
case. Thanks.



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

Reply via email to