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


##########
transaction-coordinator/src/main/java/org/apache/kafka/coordinator/transaction/TransactionMetadata.java:
##########
@@ -139,11 +139,12 @@ public TxnTransitMetadata prepareNoTransit() {
 
     public TxnTransitMetadata prepareFenceProducerEpoch() {
         if (producerEpoch == Short.MAX_VALUE)
-            throw new IllegalStateException("Cannot fence producer with epoch 
equal to Short.MaxValue since this would overflow");
+            LOGGER.error("Fencing producer {} {} with epoch equal to 
Short.MaxValue, this must not happen unless there is a bug", transactionalId, 
producerId);

Review Comment:
   The message is there so that there is trail that this scenario happened.  It 
would help to RCA weird cases.



##########
transaction-coordinator/src/main/java/org/apache/kafka/coordinator/transaction/TransactionMetadata.java:
##########
@@ -139,11 +139,12 @@ public TxnTransitMetadata prepareNoTransit() {
 
     public TxnTransitMetadata prepareFenceProducerEpoch() {
         if (producerEpoch == Short.MAX_VALUE)
-            throw new IllegalStateException("Cannot fence producer with epoch 
equal to Short.MaxValue since this would overflow");
+            LOGGER.error("Fencing producer {} {} with epoch equal to 
Short.MaxValue, this must not happen unless there is a bug", transactionalId, 
producerId);
 
         // If we've already failed to fence an epoch (because the write to the 
log failed), we don't increase it again.
         // This is safe because we never return the epoch to client if we fail 
to fence the epoch
-        short bumpedEpoch = hasFailedEpochFence ? producerEpoch : (short) 
(producerEpoch + 1);
+        // Also don't increase if producerEpoch is already at max, to avoid 
overflow.
+        short bumpedEpoch = hasFailedEpochFence || producerEpoch == 
Short.MAX_VALUE ? producerEpoch : (short) (producerEpoch + 1);

Review Comment:
   You are correct -- the bumped epoch is then ignored in TV2 code path.  But 
if we don't make changes here, the TV2 code path wouldn't get through.



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