-
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: remove NewTxn and NewStaleTxnWithStartTS in session #35885
Changes from 9 commits
2df07d2
71775f0
f128c89
0a64c39
c7ab65c
3de64fa
4036b05
8bdff90
3c9c1ec
77d942c
5779eee
a7186f2
929c91d
3ab0e78
8304069
b0c6f15
a3c6a56
cd26fd4
59fa3bd
98ffff8
fd9fc8a
0a41a9c
7fea97c
11c1d2c
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 |
---|---|---|
|
@@ -56,6 +56,15 @@ type baseTxnContextProvider struct { | |
enterNewTxnType sessiontxn.EnterNewTxnType | ||
} | ||
|
||
// prepareTxnNotConsiderSnapshotTS is used to prepare an oracle ts future even when SnapshotTS is set. | ||
func (p *baseTxnContextProvider) prepareTxnNotConsiderSnapshotTS() error { | ||
if snapshotTS := p.sctx.GetSessionVars().SnapshotTS; snapshotTS == 0 { | ||
return nil | ||
} | ||
p.isTxnPrepared = false | ||
return p.prepareTxn(false) | ||
} | ||
|
||
// OnInitialize is the hook that should be called when enter a new txn with this provider | ||
func (p *baseTxnContextProvider) OnInitialize(ctx context.Context, tp sessiontxn.EnterNewTxnType) (err error) { | ||
if p.getStmtReadTSFunc == nil || p.getStmtForUpdateTSFunc == nil { | ||
|
@@ -67,12 +76,18 @@ func (p *baseTxnContextProvider) OnInitialize(ctx context.Context, tp sessiontxn | |
activeNow := true | ||
switch tp { | ||
case sessiontxn.EnterNewTxnDefault: | ||
if err = p.sctx.NewTxn(ctx); err != nil { | ||
if err := sessiontxn.CheckBeforeNewTxn(p.ctx, p.sctx); err != nil { | ||
return err | ||
} | ||
if err := p.prepareTxnNotConsiderSnapshotTS(); err != nil { | ||
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. Is it ok if we call |
||
return err | ||
} | ||
case sessiontxn.EnterNewTxnWithBeginStmt: | ||
if !sessiontxn.CanReuseTxnWhenExplicitBegin(p.sctx) { | ||
if err = p.sctx.NewTxn(ctx); err != nil { | ||
if err := sessiontxn.CheckBeforeNewTxn(p.ctx, p.sctx); err != nil { | ||
return err | ||
} | ||
if err := p.prepareTxnNotConsiderSnapshotTS(); err != nil { | ||
return err | ||
} | ||
} | ||
|
@@ -102,7 +117,7 @@ func (p *baseTxnContextProvider) OnInitialize(ctx context.Context, tp sessiontxn | |
if err != nil { | ||
return err | ||
} | ||
p.isTxnPrepared = txn.Valid() || p.sctx.GetPreparedTxnFuture() != nil | ||
p.isTxnPrepared = txn.Valid() || p.sctx.GetPreparedTxnFuture().GetPreparedTSFuture() != nil | ||
if activeNow { | ||
_, err = p.ActivateTxn() | ||
} | ||
|
@@ -172,7 +187,7 @@ func (p *baseTxnContextProvider) ActivateTxn() (kv.Transaction, error) { | |
return p.txn, nil | ||
} | ||
|
||
if err := p.prepareTxn(); err != nil { | ||
if err := p.prepareTxn(true); err != nil { | ||
return nil, err | ||
} | ||
|
||
|
@@ -227,13 +242,15 @@ func (p *baseTxnContextProvider) ActivateTxn() (kv.Transaction, error) { | |
return txn, nil | ||
} | ||
|
||
func (p *baseTxnContextProvider) prepareTxn() error { | ||
func (p *baseTxnContextProvider) prepareTxn(considerSnapshotTS bool) error { | ||
lcwangchao marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if p.isTxnPrepared { | ||
return nil | ||
} | ||
|
||
if snapshotTS := p.sctx.GetSessionVars().SnapshotTS; snapshotTS != 0 { | ||
return p.prepareTxnWithTS(snapshotTS) | ||
if considerSnapshotTS { | ||
if snapshotTS := p.sctx.GetSessionVars().SnapshotTS; snapshotTS != 0 { | ||
return p.prepareTxnWithTS(snapshotTS) | ||
} | ||
} | ||
|
||
future := sessiontxn.NewOracleFuture(p.ctx, p.sctx, p.sctx.GetSessionVars().TxnCtx.TxnScope) | ||
|
@@ -280,7 +297,7 @@ func (p *baseTxnContextProvider) AdviseWarmup() error { | |
// When executing `START TRANSACTION READ ONLY AS OF ...` no need to warmUp | ||
return nil | ||
} | ||
return p.prepareTxn() | ||
return p.prepareTxn(true) | ||
} | ||
|
||
// AdviseOptimizeWithPlan providers optimization according to the plan | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,14 +16,17 @@ package staleread | |
|
||
import ( | ||
"context" | ||
"time" | ||
|
||
"github.com/pingcap/errors" | ||
"github.com/pingcap/tidb/config" | ||
"github.com/pingcap/tidb/infoschema" | ||
"github.com/pingcap/tidb/kv" | ||
"github.com/pingcap/tidb/parser/ast" | ||
"github.com/pingcap/tidb/sessionctx" | ||
"github.com/pingcap/tidb/sessionctx/variable" | ||
"github.com/pingcap/tidb/sessiontxn" | ||
"github.com/pingcap/tidb/table/temptable" | ||
) | ||
|
||
// StalenessTxnContextProvider implements sessiontxn.TxnContextProvider | ||
|
@@ -73,18 +76,51 @@ func (p *StalenessTxnContextProvider) OnInitialize(ctx context.Context, tp sessi | |
} | ||
|
||
func (p *StalenessTxnContextProvider) activateStaleTxn() error { | ||
if err := p.sctx.NewStaleTxnWithStartTS(p.ctx, p.ts); err != nil { | ||
var err error | ||
if err = sessiontxn.CheckBeforeNewTxn(p.ctx, p.sctx); err != nil { | ||
return err | ||
} | ||
p.is = p.sctx.GetSessionVars().TxnCtx.InfoSchema.(infoschema.InfoSchema) | ||
if err := p.sctx.GetSessionVars().SetSystemVar(variable.TiDBSnapshot, ""); err != nil { | ||
|
||
txnScope := config.GetTxnScopeFromConfig() | ||
if err = p.sctx.PrepareTSFuture(p.ctx, sessiontxn.ConstantFuture(p.ts), txnScope); err != nil { | ||
return err | ||
} | ||
|
||
txnCtx := p.sctx.GetSessionVars().TxnCtx | ||
txnCtx.IsStaleness = true | ||
txnCtx.InfoSchema = p.is | ||
return nil | ||
txnFuture := p.sctx.GetPreparedTxnFuture() | ||
if txnFuture == nil { | ||
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. ts is prepared in line 97. I think we can guarantee txnFuture is not nil here. Maybe this line is not necessary 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, it's unnecessary. |
||
return errors.AddStack(kv.ErrInvalidTxn) | ||
} | ||
|
||
txn, err := txnFuture.Wait(p.ctx, p.sctx) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
sessVars := p.sctx.GetSessionVars() | ||
txn.SetVars(sessVars.KVVars) | ||
txn.SetOption(kv.IsStalenessReadOnly, true) | ||
txn.SetOption(kv.TxnScope, txnScope) | ||
sessiontxn.SetTxnAssertionLevel(txn, sessVars.AssertionLevel) | ||
is, err := GetSessionSnapshotInfoSchema(p.sctx, p.ts) | ||
if err != nil { | ||
return errors.Trace(err) | ||
} | ||
sessVars.TxnCtx = &variable.TransactionContext{ | ||
TxnCtxNoNeedToRestore: variable.TxnCtxNoNeedToRestore{ | ||
InfoSchema: is, | ||
CreateTime: time.Now(), | ||
StartTS: txn.StartTS(), | ||
ShardStep: int(sessVars.ShardAllocateStep), | ||
IsStaleness: true, | ||
TxnScope: txnScope, | ||
}, | ||
} | ||
txn.SetOption(kv.SnapInterceptor, temptable.SessionSnapshotInterceptor(p.sctx)) | ||
|
||
p.is = is | ||
err = p.sctx.GetSessionVars().SetSystemVar(variable.TiDBSnapshot, "") | ||
|
||
return err | ||
} | ||
|
||
func (p *StalenessTxnContextProvider) enterNewStaleTxnWithReplaceProvider() error { | ||
|
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 remove this? I think it is reasonable if a txn is not prepared the
TxnFuture
should return a nil value.