-
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
txn: refactor ts acquisition within build and execute phases #35376
Changes from all commits
cc05adf
0f136bd
9ad4492
9a68f75
ddd7ed4
142f55e
ebb581f
b595d3d
673d389
0b431c2
516530b
4c832a2
5e7fc56
781ce8f
b54c368
139e506
cff38ac
b3dd7ee
3c7d1d3
0b70f0e
9a500e9
b88d5ab
7b40adc
4906089
bd12553
350efbb
2f28146
4e52ee6
4700b48
1c5a1e9
aa9c357
be1c611
83c5fa9
1da2557
251a875
fe66d11
3cfb2fa
035a2bd
0d3e267
d83319b
3be9075
f519b42
1ea7192
168f950
2eccd2e
303dbd7
26d11e3
2d4caf4
1092f63
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -49,6 +49,7 @@ import ( | |
"github.com/pingcap/tidb/sessionctx" | ||
"github.com/pingcap/tidb/sessionctx/stmtctx" | ||
"github.com/pingcap/tidb/sessionctx/variable" | ||
"github.com/pingcap/tidb/sessiontxn" | ||
"github.com/pingcap/tidb/table" | ||
"github.com/pingcap/tidb/table/tables" | ||
"github.com/pingcap/tidb/tablecodec" | ||
|
@@ -1042,12 +1043,20 @@ func (e *SelectLockExec) Next(ctx context.Context, req *chunk.Chunk) error { | |
for id := range e.tblID2Handle { | ||
e.updateDeltaForTableID(id) | ||
} | ||
|
||
return doLockKeys(ctx, e.ctx, newLockCtx(e.ctx.GetSessionVars(), lockWaitTime, len(e.keys)), e.keys...) | ||
lockCtx, err := newLockCtx(e.ctx, lockWaitTime, len(e.keys)) | ||
if err != nil { | ||
return err | ||
} | ||
return doLockKeys(ctx, e.ctx, lockCtx, e.keys...) | ||
} | ||
|
||
func newLockCtx(seVars *variable.SessionVars, lockWaitTime int64, numKeys int) *tikvstore.LockCtx { | ||
lockCtx := tikvstore.NewLockCtx(seVars.TxnCtx.GetForUpdateTS(), lockWaitTime, seVars.StmtCtx.GetLockWaitStartTime()) | ||
func newLockCtx(sctx sessionctx.Context, lockWaitTime int64, numKeys int) (*tikvstore.LockCtx, error) { | ||
seVars := sctx.GetSessionVars() | ||
forUpdateTS, err := sessiontxn.GetTxnManager(sctx).GetStmtForUpdateTS() | ||
if err != nil { | ||
return nil, err | ||
} | ||
lockCtx := tikvstore.NewLockCtx(forUpdateTS, lockWaitTime, seVars.StmtCtx.GetLockWaitStartTime()) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You changed the old code process of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The change it not due to a bug fix. This PR aims to change the management of the acquisition of ts by txnManager. Previsouly, some ts acquisition if from txnManager, but some is from TxnCtx such as here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But the logic of "seVars.TxnCtx.GetForUpdateTS()" is different from "sessiontxn.GetTxnManager(sctx).GetStmtForUpdateTS()". There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, previsouly, executor can change TxnCtx.forUpdateTS directly. Now, forUpdateTS is managed by the relevant transaction context provider and returned by GetStmtForUpdateTS. Although when forUpdateTS is changed, we also change txnCtx.forUpdateTS, now we should use GetTxnManager(sctx).GetStmtForUpdateTS() in normal code and only use TxnCtx.GetForUpdateTS() in places such as log print. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Any plan to remove the usage of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Some will be removed in the later PRs. |
||
lockCtx.Killed = &seVars.Killed | ||
lockCtx.PessimisticLockWaited = &seVars.StmtCtx.PessimisticLockWaited | ||
lockCtx.LockKeysDuration = &seVars.StmtCtx.LockKeysDuration | ||
|
@@ -1082,7 +1091,7 @@ func newLockCtx(seVars *variable.SessionVars, lockWaitTime int64, numKeys int) * | |
if lockCtx.ForUpdateTS > 0 && seVars.AssertionLevel != variable.AssertionLevelOff { | ||
lockCtx.InitCheckExistence(numKeys) | ||
} | ||
return lockCtx | ||
return lockCtx, nil | ||
} | ||
|
||
// doLockKeys is the main entry for pessimistic lock keys | ||
|
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.
Why do you change "updateForUpdateTS" to "getSnapshotTS()"?
Does it update the latest TS for "SELECT ... FOR UPDATE" in original code, but not now.
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.
getSnapshotTS
can also update for_update_ts. The naming is somewhat confusing..