-
Notifications
You must be signed in to change notification settings - Fork 5.9k
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
executor: do not read from lock cache when snapshot read (#21529) #21539
executor: do not read from lock cache when snapshot read (#21529) #21539
Conversation
Signed-off-by: ti-srebot <[email protected]>
/run-all-tests |
@you06 you're already a collaborator in bot's repo. |
Signed-off-by: you06 <[email protected]>
/run-all-tests |
Seems |
Signed-off-by: you06 <[email protected]>
/run-all-tests |
Fixed. |
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
@jebter I think we need to do some benchmark after merging this PR, maybe there will be some benchmark improvement relying on the mistakenly used
There may be some regression for this kind of benchmark. |
LGTM |
Here's the result testing with https://gist.github.com/you06/ac94678ccfb69448238e31955ebd5350
The latency reading from lock cache is much lower, however point get from tikv seems efficient enough. @cfzjywxk cc |
cherry-pick #21529 to release-4.0
Signed-off-by: you06 [email protected]
What problem does this PR solve?
Issue Number: close #21447
Problem Summary:
What is changed and how it works?
What's Changed:
When read from snapshot and key not exist in membuffer, do not look up from lock cache.
How it Works:
Lock key will write lock cache, if the key is locked but the value is not affected the locked value will only be written into lock cache, the cached value should not read by a snapshot read.
Related changes
Check List
Tests
Release note