domain: add resolve lock logic for mvcc get key loading schema diff #48282
Conversation
cfzjywxk
added
type/bugfix
cfzjywxk
requested review from
zyguan,
you06,
MyonKeminta,
crazycs520 and
ekexium
ti-chi-bot
bot
added
release-note
|
Hi @cfzjywxk. Thanks for your PR. PRs from untrusted users cannot be marked as trusted with I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
ti-chi-bot
bot
added
the
do-not-merge/work-in-progress
store/helper/helper.go
Outdated
| // GetMvccByEncodedKeyWithTS get the MVCC value by the specific encoded key, if lock is encountered it would be resolved. | ||
| func (h *Helper) GetMvccByEncodedKeyWithTS(encodedKey kv.Key, startTS uint64) (*kvrpcpb.MvccGetByKeyResponse, error) { | ||
| // A derived value from previous implementation possible experiencing value 5000ms. | ||
| MaxBackoffTimeout := 5000 |
| helper := helper.NewHelper(tikvStore) | ||
| data, err := helper.GetMvccByEncodedKey(m.EncodeSchemaDiffKey(version)) | ||
| newHelper := helper.NewHelper(tikvStore) | ||
| mvccResp, err := newHelper.GetMvccByEncodedKeyWithTS(m.EncodeSchemaDiffKey(version), startTS) |
There was a problem hiding this comment.
Can we just call another GetMvccByEncodedKey(lock.primary) to get the commit-ts? Resolving lock and retring makes GetMvcc complex and heavy.
There was a problem hiding this comment.
It's possible the primary key is not committed or the ongoing internal transaction is invisible to the input start_ts.
There was a problem hiding this comment.
Besides, it's better to use snapshot interfaces instead of the mvcc interfaces, I've filed an issue about it #48283
…ingcap#48282) Signed-off-by: crazycs520 <crazycs520@gmail.com>
ti-chi-bot
bot
removed
the
do-not-merge/work-in-progress
|
You can found some test result(record) here: #48294 |
ti-chi-bot
bot
added
needs-1-more-lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: crazycs520, zyguan The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
[LGTM Timeline notifier]Timeline:
|
cfzjywxk
merged commit e811102
into
pingcap:tidb-6.5-with-kv-timeout-feature
…ingcap#48282 (pingcap#18) * add resolve lock logic for mvcc get key loading schema diff * adstract a constant Co-authored-by: cfzjywxk <cfzjywxk@gmail.com>
What problem does this PR solve?
Issue Number: ref #48281
Problem Summary:
Handle lock error reading mvcc get schema diff key to avoid unexpected schema cache invalidation.
What is changed and how it works?
Check List
Tests
Side effects
Documentation
Release note
Please refer to Release Notes Language Style Guide to write a quality release note.