-
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: Provide PessimisticRCTxnContextProvider
for RC isolation
#34702
Changes from 43 commits
2a06641
6ef49e2
d9e4ab4
2b27055
7f3cdd4
2cac30a
c1d50ce
33017e8
cf3ca7c
9644438
8a22474
6bcaed7
38391fc
de87671
b45839c
4ae29f8
344012f
7ba70d0
9534210
67d3117
66d508a
283226e
438073e
0e3eab8
90e8241
76eadb7
c71ff52
be296b2
ce409f9
6f5d6ef
71e8a93
fd93a26
c68a89a
754df88
d0c839e
c8b48e3
9694ad6
467962a
bd3ec9b
6d8954b
f8d20d1
b9229ef
f1a7861
4d9429f
9cf9485
1755975
e999728
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 |
---|---|---|
|
@@ -62,7 +62,6 @@ import ( | |
"github.com/pingcap/tidb/util/collate" | ||
"github.com/pingcap/tidb/util/cteutil" | ||
"github.com/pingcap/tidb/util/execdetails" | ||
"github.com/pingcap/tidb/util/logutil" | ||
"github.com/pingcap/tidb/util/mathutil" | ||
"github.com/pingcap/tidb/util/memory" | ||
"github.com/pingcap/tidb/util/ranger" | ||
|
@@ -71,7 +70,6 @@ import ( | |
"github.com/pingcap/tidb/util/timeutil" | ||
"github.com/pingcap/tipb/go-tipb" | ||
"github.com/tikv/client-go/v2/tikv" | ||
"go.uber.org/zap" | ||
) | ||
|
||
var ( | ||
|
@@ -1584,11 +1582,6 @@ func (b *executorBuilder) getSnapshotTS() (uint64, error) { | |
// and some stale/historical read contexts. For example, it will return txn.StartTS in RR and return | ||
// the current timestamp in RC isolation | ||
func (b *executorBuilder) getReadTS() (uint64, error) { | ||
// `refreshForUpdateTSForRC` should always be invoked before returning the cached value to | ||
// ensure the correct value is returned even the `snapshotTS` field is already set by other | ||
// logics. However for `IndexLookUpMergeJoin` and `IndexLookUpHashJoin`, it requires caching the | ||
// snapshotTS and and may even use it after the txn being destroyed. In this case, mark | ||
// `snapshotTSCached` to skip `refreshForUpdateTSForRC`. | ||
failpoint.Inject("assertNotStaleReadForExecutorGetReadTS", func() { | ||
// after refactoring stale read will use its own context provider | ||
staleread.AssertStmtStaleness(b.ctx, false) | ||
|
@@ -1604,12 +1597,6 @@ func (b *executorBuilder) getReadTS() (uint64, error) { | |
return snapshotTS, nil | ||
} | ||
|
||
if b.ctx.GetSessionVars().IsPessimisticReadConsistency() { | ||
if err := b.refreshForUpdateTSForRC(); err != nil { | ||
return 0, err | ||
} | ||
} | ||
|
||
if b.snapshotTS != 0 { | ||
b.snapshotTSCached = true | ||
// Return the cached value. | ||
|
@@ -2229,41 +2216,12 @@ func (b *executorBuilder) updateForUpdateTSIfNeeded(selectPlan plannercore.Physi | |
// The Repeatable Read transaction use Read Committed level to read data for writing (insert, update, delete, select for update), | ||
// We should always update/refresh the for-update-ts no matter the isolation level is RR or RC. | ||
if b.ctx.GetSessionVars().IsPessimisticReadConsistency() { | ||
return b.refreshForUpdateTSForRC() | ||
_, err = sessiontxn.GetTxnManager(b.ctx).GetStmtForUpdateTS() | ||
return err | ||
} | ||
return UpdateForUpdateTS(b.ctx, 0) | ||
} | ||
|
||
// refreshForUpdateTSForRC is used to refresh the for-update-ts for reading data at read consistency level in pessimistic transaction. | ||
// It could use the cached tso from the statement future to avoid get tso many times. | ||
func (b *executorBuilder) refreshForUpdateTSForRC() error { | ||
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. Ditto, would the 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, I want to remove it in the future. But we should still keep it when all codes are moved to provider. Because for Maybe another way is to add a new method |
||
defer func() { | ||
b.snapshotTS = b.ctx.GetSessionVars().TxnCtx.GetForUpdateTS() | ||
}() | ||
// The first time read-consistency read is executed and `RcReadCheckTS` is enabled, try to use | ||
// the last valid ts as the for update read ts. | ||
if b.ctx.GetSessionVars().StmtCtx.RCCheckTS { | ||
rcReadTS := b.ctx.GetSessionVars().TxnCtx.LastRcReadTs | ||
if rcReadTS == 0 { | ||
rcReadTS = b.ctx.GetSessionVars().TxnCtx.StartTS | ||
} | ||
return UpdateForUpdateTS(b.ctx, rcReadTS) | ||
} | ||
future := b.ctx.GetSessionVars().TxnCtx.GetStmtFutureForRC() | ||
if future == nil { | ||
return nil | ||
} | ||
newForUpdateTS, waitErr := future.Wait() | ||
if waitErr != nil { | ||
logutil.BgLogger().Warn("wait tso failed", | ||
zap.Uint64("startTS", b.ctx.GetSessionVars().TxnCtx.StartTS), | ||
zap.Error(waitErr)) | ||
} | ||
b.ctx.GetSessionVars().TxnCtx.SetStmtFutureForRC(nil) | ||
// If newForUpdateTS is 0, it will force to get a new for-update-ts from PD. | ||
return UpdateForUpdateTS(b.ctx, newForUpdateTS) | ||
} | ||
|
||
func (b *executorBuilder) buildAnalyzeIndexPushdown(task plannercore.AnalyzeIndexTask, opts map[ast.AnalyzeOptionType]uint64, autoAnalyze string) *analyzeTask { | ||
job := &statistics.AnalyzeJob{DBName: task.DBName, TableName: task.TableName, PartitionName: task.PartitionName, JobInfo: autoAnalyze + "analyze index " + task.IndexInfo.Name.O} | ||
_, offset := timeutil.Zone(b.ctx.GetSessionVars().Location()) | ||
|
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.
Will this
getReadTS
be refactored in the future so that thets
is returned from some statement provider or context provider? There's also aGetStmtReadTS
in the transaction manager.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.
Yes, after refactor completed, this will be removed