artemlivshits commented on code in PR #17402:
URL: https://github.com/apache/kafka/pull/17402#discussion_r1807057768
##########
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:
I don't think this logic is correct (in all places). As currently written
with v2 we'll fall through to transition to fatal error. We still want to
transition to abortable error just without epoch bumping. So the logic should
look like this (in all places).
```
if (canBumpEpoch()) {
if (!isTransactionV2Enabled) epochBumpRequired = true;
// ... whatever logic was there unchanged ...
} else {
// ... whatever logic was there unchanged ...
}
```
but instead of writing this in all places refactor into method
`maybeEpochBumpRequired` In that method we can have a comment that explains
why we don't need an explicit epoch bump from the client: with v2 the epoch
will get bumped on abort automatically.
--
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]
