-
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
server: add tidb_enable_shared_lock_promotion to support for share lock upgrade #55023
Conversation
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-sigs/prow repository. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #55023 +/- ##
================================================
+ Coverage 72.8630% 74.8339% +1.9708%
================================================
Files 1570 1570
Lines 439501 441345 +1844
================================================
+ Hits 320234 330276 +10042
+ Misses 99523 90819 -8704
- Partials 19744 20250 +506
Flags with carried forward coverage won't be shown. Click here to find out more.
|
pkg/sessionctx/variable/tidb_vars.go
Outdated
|
||
// TiDBEnableSharedLockUpgrade indicates whether the `select for share` statement would be executed | ||
// as `select for update` statements which do acquire pessimistic locks. | ||
TiDBEnableSharedLockUpgrade = "tidb_enable_shared_lock_upgrade" |
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
f431b78
to
27d3910
Compare
/approve |
0f62d9a
to
eb9160f
Compare
eb9160f
to
589c5bc
Compare
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.
we need to claim tidb_enable_shared_lock_promotion
will override tidb_enable_noop_functions
in release notes.
others lgtm.
|
||
// TiDBEnableSharedLockPromotion indicates whether the `select for share` statement would be executed | ||
// as `select for update` statements which do acquire pessimistic locks. | ||
TiDBEnableSharedLockPromotion = "tidb_enable_shared_lock_promotion" |
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.
Should we add this new variable into the Plan Cache Key? Otherwise the optimizer might reuse an old plan after switching this variable, which might cause some problem.
… to for update Signed-off-by: cfzjywxk <[email protected]>
Signed-off-by: cfzjywxk <[email protected]>
Signed-off-by: cfzjywxk <[email protected]>
Signed-off-by: cfzjywxk <[email protected]>
Signed-off-by: cfzjywxk <[email protected]>
Signed-off-by: cfzjywxk <[email protected]>
Signed-off-by: cfzjywxk <[email protected]>
Signed-off-by: cfzjywxk <[email protected]>
589c5bc
to
5715a91
Compare
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 for the optimizer part
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: BornChanger, easonn7, qw4990 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:
|
/retest |
@cfzjywxk: Cannot trigger testing until a trusted user reviews the PR and leaves an In response to this:
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-sigs/prow repository. |
What problem does this PR solve?
Issue Number: close #55022
Problem Summary:
tidb_enable_shared_lock_promotion
to achive that thefor share
statements could be executed likefor update
statements which acquire pessimistic locks.tidb_enable_noop_functions
is enabled, the behaviours ofselect for share
are unexpected, including:nowait
is used, the execution is notno-op
actually.tidb_enable_noop_functions
is set tooff
, theselect for share
DOES acquire locks, so ison
.tidb_enable_noop_functions
,tidb_enabled_shared_lock_promotion
andfor share
statements for executions withPointGet
andCoprocessor
pathes.What changed and how does it work?
Check List
Tests
Side effects
Documentation
Release note
Please refer to Release Notes Language Style Guide to write a quality release note.