-
Notifications
You must be signed in to change notification settings - Fork 5.8k
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
Isolation: Refactor resetting RCReadCheckTS internal flag #38174
Isolation: Refactor resetting RCReadCheckTS internal flag #38174
Conversation
Signed-off-by: TonsnakeLin <lpbgytong@163.com>
[REVIEW NOTIFICATION] This pull request has been approved by:
To complete the pull request process, please ask the reviewers in the list to review by filling The full list of commands accepted by this bot can be found here. Reviewer can indicate their review by submitting an approval review. |
…r_rc_read_checkts
Signed-off-by: TonsnakeLin <lpbgytong@163.com>
server/tidb_test.go
Outdated
require.Equal(t, 1, count) | ||
|
||
tk.MustExec("drop table t1") | ||
require.NoError(t, failpoint.Disable("github.com/pingcap/tidb/sessiontxn/isolation/CallOnStmtRetry")) |
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.
Better to disable the failpoint in a defer func
to avoid affecting other concurrent cases if an error happens.
Signed-off-by: TonsnakeLin <lpbgytong@163.com>
The change itself looks good. I'm jsut wondering how you will tell the reason of a statement retry. |
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
I mentioned it at PR #37226 |
@@ -52,6 +52,9 @@ var TsoWaitCount stringutil.StringerStr = "tsoWaitCount" | |||
// TsoUseConstantCount is the key for constant tso counter | |||
var TsoUseConstantCount stringutil.StringerStr = "tsoUseConstantCount" | |||
|
|||
// CallOnStmtRetryCount is the key for recording calling OnStmtRetry at RC isolation level | |||
var CallOnStmtRetryCount stringutil.StringerStr = "callOnStmtRetryCount" | |||
|
|||
// AssertLockErr is used to record the lock errors we encountered | |||
// Only for test | |||
var AssertLockErr stringutil.StringerStr = "assertLockError" |
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.
Caen they be defined as const variables?
I See. But the write conflict error is unavailable in |
Yes, I I realized this. Maybe it's not suitable to to it in function |
/run-check-dev |
…r_rc_read_checkts
/run-check_dev |
…r_rc_read_checkts Signed-off-by: TonsnakeLin <lpbgytong@163.com>
/run-unit-test |
@cfzjywxk please merge it ,thank you. |
/merge |
This pull request has been accepted and is ready to merge. Commit hash: fc0a435
|
TiDB MergeCI notify🔴 Bad News! New failing [1] after this pr merged.
|
Signed-off-by: TonsnakeLin lpbgytong@163.com
What problem does this PR solve?
Issue Number: Ref #37226
Problem Summary:
What is changed and how it works?
In the past, TiDB reset the internal flag
PessimisticRCTxnContextProvider::latestOracleTSValid
in function(p *PessimisticRCTxnContextProvider) handleAfterQueryError
and(p *PessimisticRCTxnContextProvider) handleAfterPessimisticLockError
. This PR moved it to(p *PessimisticRCTxnContextProvider) OnStmtRetry
, so that we can statistic the retry count because ofRCCheckTS
easily.Check List
Tests
Side effects
Documentation
Release note
Please refer to Release Notes Language Style Guide to write a quality release note.