frankvicky commented on code in PR #23663:
URL: https://github.com/apache/kafka/pull/23663#discussion_r4156427527


##########
clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordAccumulator.java:
##########
@@ -1010,6 +1010,18 @@ private List<ProducerBatch> 
drainBatchesForOneNode(MetadataSnapshot metadataSnap
                 if (shouldBackoff(first.hasLeaderChangedForTheOngoingRetry(), 
first, first.waitedTimeMs(now)))
                     continue;
 
+                // Don't assign a sequence to a new batch while doing so could 
push the partition's oldest in-flight
+                // batch out of the broker's deduplication window. 
`max.in.flight.requests.per.connection` does not
+                // bound this on its own: across a leader change a batch can 
still be in flight to the previous
+                // leader while later batches are drained to, and acknowledged 
by, the current leader. Once enough
+                // of them have been appended, the broker forgets the oldest 
batch and its retry fails with
+                // OUT_OF_ORDER_SEQUENCE. Retries are exempt because they 
already have a sequence and are already
+                // counted, and they are serialised below by the 
`firstInFlightSequence` check.
+                if (transactionManager != null && !first.hasSequence()
+                        && 
transactionManager.wouldExceedBrokerDeduplicationWindow(tp)) {
+                    continue;

Review Comment:
   The spin only happens during a leader failover and is bounded by 
`request.timeout.ms`. 
   If we want to fix it, it should cover all three conditions together in a 
follow-up.



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