rreddy-22 commented on code in PR #17402:
URL: https://github.com/apache/kafka/pull/17402#discussion_r1803565613


##########
clients/src/main/java/org/apache/kafka/clients/producer/internals/TransactionManager.java:
##########
@@ -1568,6 +1571,17 @@ public void handleResponse(AbstractResponse response) {
             Errors error = endTxnResponse.error();
 
             if (error == Errors.NONE) {
+                // For transaction version 5+, the broker includes the 
producerId and producerEpoch in the EndTxnResponse.
+                // KIP-890 Part 2 mandates bumping the epoch after every 
transaction. If the epoch overflows,
+                // a new producerId is returned with epoch set to 0.
+                if (isTransactionV2Enabled) {

Review Comment:
   > so we should never get an older version response when 
isTransactionV2Enabled == true
   Makes sense, this was my understanding as well, thanks for clarifying. So 
essentially it shouldn't be possible but just to be safe we're gonna use the 
response version to double check if we can bump the epoch instead of relying on 
the flag since that behavior could be subject to change and cause problems 
right?



##########
clients/src/main/java/org/apache/kafka/clients/producer/internals/TransactionManager.java:
##########
@@ -1568,6 +1571,17 @@ public void handleResponse(AbstractResponse response) {
             Errors error = endTxnResponse.error();
 
             if (error == Errors.NONE) {
+                // For transaction version 5+, the broker includes the 
producerId and producerEpoch in the EndTxnResponse.
+                // KIP-890 Part 2 mandates bumping the epoch after every 
transaction. If the epoch overflows,
+                // a new producerId is returned with epoch set to 0.
+                if (isTransactionV2Enabled) {

Review Comment:
   > so we should never get an older version response when 
isTransactionV2Enabled == true
   
   Makes sense, this was my understanding as well, thanks for clarifying. So 
essentially it shouldn't be possible but just to be safe we're gonna use the 
response version to double check if we can bump the epoch instead of relying on 
the flag since that behavior could be subject to change and cause problems 
right?



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