artemlivshits commented on code in PR #16332:
URL: https://github.com/apache/kafka/pull/16332#discussion_r1640809221


##########
clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java:
##########
@@ -1257,6 +1261,9 @@ public void flush() {
         this.sender.wakeup();
         try {
             this.accumulator.awaitFlushCompletion();
+            if (transactionManager != null) {
+                transactionManager.maybeClearLastError();

Review Comment:
   We need to throw the error after clearing it.  This way we don't swallow the 
error that otherwise would've been thrown in commit.  This way:
   
   - send (got error)
   - commit
   
   sequence fails if sends got any errors, and so does
   
   - send (got error)
   - flush
   - commit
   
   (with your change the latter sequence would just swallow the error, which 
makes it too easy to write buggy code for basic cases).
   
   The advantage of new logic is that if someone wants to do advanced error 
handling and ignore some errors they'll be able to:
   
   - send
   - try { flush } catch { decide }
   - commit
   
   would work for advanced cases, without swallowing errors in basic cases.



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