artemlivshits commented on code in PR #17402:
URL: https://github.com/apache/kafka/pull/17402#discussion_r1807109604
##########
clients/src/main/java/org/apache/kafka/clients/producer/internals/TransactionManager.java:
##########
@@ -762,7 +764,7 @@ synchronized void maybeResolveSequences() {
// For the transactional producer, we bump the epoch
if possible, otherwise we transition to a fatal error
String unackedMessagesErr = "The client hasn't
received acknowledgment for some previously " +
"sent messages and can no longer retry them. ";
- if (canBumpEpoch()) {
+ if (!isTransactionV2Enabled && canBumpEpoch()) {
Review Comment:
Agree on figuring out a better name, we now have many levels for supporting
epoch bumps in the system! My understanding is that canBumpEpoch checks
whether the broker supports bumping epoch via calling InitProducerId. And
transactionsV2Enabled means that broker supports bumping epoch on each
transaction (which is the next level of epoch bumping that eliminates the need
of manual epoch bumping).
canAbort is probably fine (without epoch bump support in some form we really
cannot abort transactions without risk of getting zombies). For idempotent
producer, though, it doesn't make sense, but maybe we should go through the
code and don't use this check for code paths for idempotent producer. Basically
- rename canBumpEpoch to canAbort and remove the logic for idempotent
producer from that method
- modify all places where canBumpEpoch is called for idempotent producer to
not call (because it's always true for idempotent)
This makes me think (and I know -- it was me who suggested to change the
epoch bumping change in this PR, but I didn't realize it becomes tricky) maybe
we should remove manual epoch bumping in a separate PR to keep this PR small
and unblock other work.
--
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
