KAFKA-18464: Empty Abort Transaction can fence producer incorrectly with Transactions V2 #18467
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jeffkbkim
reviewed
Jan 10, 2025
Comment on lines
+819
to
+820
| else if (!isValidEpoch) | ||
| Left(Errors.PRODUCER_FENCED) |
There was a problem hiding this comment.
please confirm my understanding:
the mentioned commit/abort + empty abort scenario currently results in fencing the empty abort because the previous commit/abort is left in progress. This change returns concurrent_transactions when performing the empty abort instead. This enables the producer to retry instead of hard failing. Is this correct?
There was a problem hiding this comment.
exactly! We will retry until the previous operation completes and then we can handle the abort. 👍
jolshan
added a commit
that referenced
this pull request
Jan 11, 2025
…ith Transactions V2 (#18467) To avoid self-fencing in the commit/abort + empty abort scenario, return the concurrent transactions error when we have pending state and do the epoch check second. In this scenario, we will complete the previous transaction before proceeding to the empty abort. Added a test that failed before the change. Note -- only the pending state is checked earlier. This is because we don’t return from EndTxn (the first commit) until we already written to the log, transitioned to PrepareX, and have the pending CompleteX state. We don't need to worry about the cases of an EndTxn request coming in with PrepareX without the pending state because that would be an older request and/or retry which are already covered. Reviewers: Artem Livshits <[email protected]>, Jeff Kim <[email protected]>
pranavt84
pushed a commit
to pranavt84/kafka
that referenced
this pull request
Jan 27, 2025
…ith Transactions V2 (apache#18467) To avoid self-fencing in the commit/abort + empty abort scenario, return the concurrent transactions error when we have pending state and do the epoch check second. In this scenario, we will complete the previous transaction before proceeding to the empty abort. Added a test that failed before the change. Note -- only the pending state is checked earlier. This is because we don’t return from EndTxn (the first commit) until we already written to the log, transitioned to PrepareX, and have the pending CompleteX state. We don't need to worry about the cases of an EndTxn request coming in with PrepareX without the pending state because that would be an older request and/or retry which are already covered. Reviewers: Artem Livshits <[email protected]>, Jeff Kim <[email protected]>
airlock-confluentinc bot
pushed a commit
to confluentinc/kafka
that referenced
this pull request
Jan 27, 2025
…ith Transactions V2 (apache#18467) To avoid self-fencing in the commit/abort + empty abort scenario, return the concurrent transactions error when we have pending state and do the epoch check second. In this scenario, we will complete the previous transaction before proceeding to the empty abort. Added a test that failed before the change. Note -- only the pending state is checked earlier. This is because we don’t return from EndTxn (the first commit) until we already written to the log, transitioned to PrepareX, and have the pending CompleteX state. We don't need to worry about the cases of an EndTxn request coming in with PrepareX without the pending state because that would be an older request and/or retry which are already covered. Reviewers: Artem Livshits <[email protected]>, Jeff Kim <[email protected]>
manoj-mathivanan
pushed a commit
to manoj-mathivanan/kafka
that referenced
this pull request
Feb 19, 2025
…ith Transactions V2 (apache#18467) To avoid self-fencing in the commit/abort + empty abort scenario, return the concurrent transactions error when we have pending state and do the epoch check second. In this scenario, we will complete the previous transaction before proceeding to the empty abort. Added a test that failed before the change. Note -- only the pending state is checked earlier. This is because we don’t return from EndTxn (the first commit) until we already written to the log, transitioned to PrepareX, and have the pending CompleteX state. We don't need to worry about the cases of an EndTxn request coming in with PrepareX without the pending state because that would be an older request and/or retry which are already covered. Reviewers: Artem Livshits <[email protected]>, Jeff Kim <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
To avoid self-fencing in the commit/abort + empty abort scenario, return the concurrent transactions error when we have pending state and do the epoch check second. In this scenario, we will complete the previous transaction before proceeding to the empty abort.
Added a test that failed before the change.
Note -- only the pending state is checked earlier. This is because we don’t return from EndTxn (the first commit) until we already written to the log, transitioned to PrepareX, and have the pending CompleteX state. We don't need to worry about the cases of an EndTxn request coming in with PrepareX without the pending state because that would be an older request and/or retry which are already covered.