chia7712 commented on code in PR #21176:
URL: https://github.com/apache/kafka/pull/21176#discussion_r2641844371
##########
storage/src/main/java/org/apache/kafka/storage/internals/log/ProducerAppendInfo.java:
##########
@@ -119,6 +119,21 @@ private void checkProducerEpoch(short producerEpoch, long
offset, short transact
boolean invalidEpoch = (transactionVersion >= 2) ? (producerEpoch <=
current) : (producerEpoch < current);
if (invalidEpoch) {
+ // TV2 Idempotent Retry Detection:
+ // When markerEpoch == currentEpoch and no transaction is ongoing,
this is a retry
+ // of a marker that was already successfully written. Common
scenarios:
+ // 1. Coordinator recovery: reloading PREPARE_COMMIT/ABORT from
the transaction log
+ // 2. Network retry: marker was written but response was lost due
to disconnection
+ // In both cases, the transaction has already ended
(currentTxnFirstOffset is empty),
+ // so we can safely treat this as idempotent success.
+ if (transactionVersion >= 2 &&
+ producerEpoch == current &&
+ updatedEntry.currentTxnFirstOffset().isEmpty()) {
+ log.debug("Received duplicate transaction marker for producer
{} with epoch {} " +
+ "but transaction is no longer ongoing,
treating as idempotent success",
+ producerId, producerEpoch);
+ throw new IdempotentTransactionMarkerException();
Review Comment:
I agree that using exception for control is an anti-pattern, especially for
success scenarios.
However, the write path for the offset topic appears to lack built-in
idempotency logic, so avoiding the exception approach might require more
refactoring. Regardless, I agree it is worth the effort to implement this
without relying on exceptions
--
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]
