-
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
*: Txn() function signature refactor and remove ActivePendingTxn() (#8327) #8567
Conversation
…ingcap#8327) * change Txn() function signature to Txn(active bool) * ActivePendingTxn() is not used any more because Txn() does the work * change executor builder getStartTS() uint64 to getStartTS() (uint64, error)
/run-all-tests tidb-test=pr/674 |
@@ -125,7 +125,7 @@ func (e *CheckIndexRangeExec) Open(ctx context.Context) error { | |||
|
|||
func (e *CheckIndexRangeExec) buildDAGPB() (*tipb.DAGRequest, error) { | |||
dagReq := &tipb.DAGRequest{} | |||
dagReq.StartTs = e.ctx.Txn().StartTS() | |||
dagReq.StartTs = e.ctx.Txn(true).StartTS() |
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 check if sctx.Txn(true).Valid()
first?
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
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
What problem does this PR solve?
Cherry pick from #8327
What is changed and how it works?
I thought #8327 is just a simple refactor, so I did not cherry pick that to release 2.1
But later it turns out that without the commit, release 2.1 will fail on this test #8566
@crazycs520 @winkyao
This change is