Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

KAFKA-14055; Txn markers should not be removed by matching records in the offset map #12390

Merged
merged 1 commit into from Jul 10, 2022

Conversation

hachikuji
Copy link
Contributor

@hachikuji hachikuji commented Jul 7, 2022

When cleaning a topic with transactional data, if the keys used in the user data happen to conflict with the keys in the transaction markers, it is possible for the markers to get removed before the corresponding data from the transaction is removed. This results in a hanging transaction or the loss of the transaction's atomicity since it would effectively get bundled into the next transaction in the log. Currently control records are excluded when building the offset map, but not when doing the cleaning. This patch fixes the problem by checking for control batches in the shouldRetainRecord callback.

Committer Checklist (excluded from commit message)

  • Verify design and implementation
  • Verify test coverage and CI build status
  • Verify documentation (including upgrade notes)

Copy link
Contributor

@junrao junrao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hachikuji : Thanks for the PR. LGTM. Are the test timeouts related to this PR?

@hachikuji
Copy link
Contributor Author

@junrao Thanks for reviewing. The ARM agent has been a bit unreliable. It looks like the ARM node has been offline for a while. There is some discussion here about removing it from the PR build.

@hachikuji hachikuji merged commit 0bc8da7 into apache:trunk Jul 10, 2022
hachikuji added a commit that referenced this pull request Jul 10, 2022
… the offset map (#12390)

When cleaning a topic with transactional data, if the keys used in the user data happen to conflict with the keys in the transaction markers, it is possible for the markers to get removed before the corresponding data from the transaction is removed. This results in a hanging transaction or the loss of the transaction's atomicity since it would effectively get bundled into the next transaction in the log. Currently control records are excluded when building the offset map, but not when doing the cleaning. This patch fixes the problem by checking for control batches in the `shouldRetainRecord` callback.

Reviewers: Jun Rao <junrao@gmail.com>
hachikuji added a commit that referenced this pull request Jul 10, 2022
… the offset map (#12390)

When cleaning a topic with transactional data, if the keys used in the user data happen to conflict with the keys in the transaction markers, it is possible for the markers to get removed before the corresponding data from the transaction is removed. This results in a hanging transaction or the loss of the transaction's atomicity since it would effectively get bundled into the next transaction in the log. Currently control records are excluded when building the offset map, but not when doing the cleaning. This patch fixes the problem by checking for control batches in the `shouldRetainRecord` callback.

Reviewers: Jun Rao <junrao@gmail.com>
hachikuji added a commit that referenced this pull request Jul 10, 2022
… the offset map (#12390)

When cleaning a topic with transactional data, if the keys used in the user data happen to conflict with the keys in the transaction markers, it is possible for the markers to get removed before the corresponding data from the transaction is removed. This results in a hanging transaction or the loss of the transaction's atomicity since it would effectively get bundled into the next transaction in the log. Currently control records are excluded when building the offset map, but not when doing the cleaning. This patch fixes the problem by checking for control batches in the `shouldRetainRecord` callback.

Reviewers: Jun Rao <junrao@gmail.com>
ijuma added a commit to confluentinc/kafka that referenced this pull request Aug 3, 2022
> $ git merge-base apache-github/3.3 apache-github/trunk
> 23c92ce

> $ git show 23c92ce
> commit 23c92ce
> Author: SC <pch838811@gmail.com>
> Date:   Mon Jul 11 11:36:56 2022 +0900
>
>    MINOR: Use String#format for niceMemoryUnits result (apache#12389)
>
>    Reviewers: Luke Chen <showuon@gmail.com>, Divij Vaidya <diviv@amazon.com>

* commit '23c92ce79366e86ca719e5e51c550c27324acd83':
  MINOR: Use String#format for niceMemoryUnits result (apache#12389)
  KAFKA-14055; Txn markers should not be removed by matching records in the offset map (apache#12390)
  KAFKA-13474: Allow reconfiguration of SSL certs for broker to controller connection (apache#12381)
  KAFKA-13996: log.cleaner.io.max.bytes.per.second can be changed dynamically (apache#12296)
  KAFKA-13983: Fail the creation with "/" in resource name in zk ACL (apache#12359)
  KAFKA-12943: update aggregating documentation (apache#12091)
  KAFKA-13846: Follow up PR to address review comments (apache#12297)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants