-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
[Transaction] add method to clear up transaction buffer snapshot #11934
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
txn.commit().get(); | ||
|
||
// wait for take snapshot | ||
Thread.sleep(1000 * 3); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we trigger the snapshot manually? this might introduce a flaky test in the CI enviroment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I'll fix this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@codelipenghui why you added 2.8.2 label ?
This looks like a new (internal) feature.
We should not port commits to a "stable" branch if not strictly needed. Otherwise we risk to compromise stability.
) (cherry picked from commit d86db3f)
Motivation
Currently, the transaction buffer didn't support clear up the snapshot, if delete the topic the snapshot will not be deleted.
refer to #11928
Modifications
Clear up the transaction buffer snapshot when deleting the topic.
Verifying this change
Add a test to verify the snapshot was clear up or not.
Does this pull request potentially affect one of the following parts:
If
yes
was chosen, please highlight the changesDocumentation
Check the box below and label this PR (if you have committer privilege).
Need to update docs?
doc-required
(If you need help on updating docs, create a doc issue)
no-need-doc
(Please explain why)
doc
(If this PR contains doc changes)