Skip to content

Conversation

@bandalgomsu
Copy link
Contributor

Refactor fixTxOffsets to use nextOffsets API to correctly skip transactional commit markers
issue : #1587

thanks 🙂

@sobychacko
Copy link
Contributor

@bandalgomsu Thanks for the PR! Given our proximity to GA release, we'd prefer to defer this merge to avoid last-minute regressions. We'll revisit after next week's release.

Meanwhile, could you add either a detailed commit message explaining the motivation or inline code comments? We'll also need tests to validate these changes.

We need to make sure that the feedback that was given here is addressed adequately: #1587 (comment)

Replaced the position()-based offset correction with nextOffsets() to
address known issues with transactional topics:
 - Works correctly with both read_committed and read_uncommitted
 - Handles empty poll() advancement and leader epoch propagation
 - Fixes the case when max.poll.records equals transaction batch size

Signed-off-by: Su Ko <rhtn1128@gmail.com>
@bandalgomsu
Copy link
Contributor Author

Thanks for the comments !!
i added it 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants