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

implement rollback for pipelined dml #1235

Merged
merged 11 commits into from
Apr 9, 2024
Merged

Conversation

you06
Copy link
Contributor

@you06 you06 commented Mar 18, 2024

ref tikv/tikv#16291

Now the locks of rollback pipelined DML need to be cleaned up by GC. With this PR, the locks can be cleanup faster.

Flush phase

image

Rollback phase

image

SQL client

image

Signed-off-by: you06 <you1474600@gmail.com>
@you06 you06 requested review from cfzjywxk and ekexium March 18, 2024 06:37
Signed-off-by: you06 <you1474600@gmail.com>
Signed-off-by: you06 <you1474600@gmail.com>
// TODO: implement cleanup.
// resolveFlushedLocks resolves all locks in the given range [start, end) with the given status.
// The resolve process is running in another goroutine so this function won't block.
func (c *twoPhaseCommitter) resolveFlushedLocks(bo *retry.Backoffer, start, end []byte, commit bool) {
const RESOLVE_CONCURRENCY = 8
var resolved atomic.Uint64
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure if we need to choose lower concurrency for rollback.

Signed-off-by: you06 <you1474600@gmail.com>

address comment

Signed-off-by: you06 <you1474600@gmail.com>
@@ -334,6 +335,18 @@ func (c *twoPhaseCommitter) buildPipelinedResolveHandler(commit bool, resolved *
maxBackOff = CommitSecondaryMaxBackoff
}
regionCache := c.store.GetRegionCache()
// the handler function runs in a different goroutine, should copy the required values before it to avoid race.
Copy link
Contributor

Choose a reason for hiding this comment

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

As dicussed in above comments, this may not be a perfect solution, we still need to find a way to avoid race completely.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe set txn.committer to nil which disable further accessing to rollbacked committer.

@you06 you06 removed the hold label Apr 8, 2024
@cfzjywxk cfzjywxk merged commit 714958c into tikv:master Apr 9, 2024
10 checks passed
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.

3 participants