-
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
tikv/txn: support local latch in transaction #6418
Changes from 14 commits
656dd7b
b51c483
ed8154b
41dc21c
2678449
25a893f
4a8e29b
cb90239
2502227
6f3263b
9b465c4
25faf0d
7657fa4
e8c279b
c6a0002
0c5caaa
37695a6
79fdafd
47d6420
f522381
1c0ab63
40da727
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 |
---|---|---|
|
@@ -107,12 +107,22 @@ const ( | |
LastExecuteDDL basicCtxType = 3 | ||
) | ||
|
||
type contextKey string | ||
|
||
// ConnID is the key in context. | ||
const ConnID contextKey = "conn ID" | ||
const ConnID kv.ContextKey = "conn ID" | ||
|
||
// SetCommitCtx sets the variables for context before commit a transaction. | ||
func SetCommitCtx(ctx context.Context, sessCtx Context) context.Context { | ||
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. Pass such important information through 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 suggestion? |
||
ctx = context.WithValue(ctx, ConnID, sessCtx.GetSessionVars().ConnectionID) | ||
retryAble := !sessCtx.GetSessionVars().TxnCtx.ForUpdate | ||
return context.WithValue(ctx, kv.Retryable, retryAble) | ||
} | ||
|
||
// SetConnID2Ctx sets the connection ID to context. | ||
func SetConnID2Ctx(ctx context.Context, sessCtx Context) context.Context { | ||
return context.WithValue(ctx, ConnID, sessCtx.GetSessionVars().ConnectionID) | ||
// GetRetryable returns the value of Retryable from the ctx. | ||
func GetRetryable(ctx context.Context) bool { | ||
var retryable bool | ||
val := ctx.Value(kv.Retryable) | ||
if val != nil { | ||
retryable = val.(bool) | ||
} | ||
return retryable | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -28,7 +28,7 @@ type LatchesScheduler struct { | |
} | ||
|
||
// NewScheduler create the LatchesScheduler. | ||
func NewScheduler(size int) *LatchesScheduler { | ||
func NewScheduler(size uint) *LatchesScheduler { | ||
latches := NewLatches(size) | ||
unlockCh := make(chan *Lock, lockChanSize) | ||
scheduler := &LatchesScheduler{ | ||
|
@@ -84,7 +84,7 @@ func (scheduler *LatchesScheduler) Lock(startTS uint64, keys [][]byte) *Lock { | |
} | ||
|
||
// UnLock unlocks a lock with commitTS. | ||
func (scheduler *LatchesScheduler) UnLock(lock *Lock, commitTS uint64) { | ||
func (scheduler *LatchesScheduler) UnLock(lock *Lock) { | ||
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. IMHO, this API design is worse than before, because the user may forget to call 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. Both are ok for me. It's suggested by @coocood , maybe I could update it when you reach an agreement? |
||
scheduler.RLock() | ||
defer scheduler.RUnlock() | ||
if !scheduler.closed { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,7 +22,6 @@ import ( | |
"github.com/pingcap/tidb/kv" | ||
"github.com/pingcap/tidb/metrics" | ||
"github.com/pingcap/tidb/sessionctx" | ||
binlog "github.com/pingcap/tipb/go-binlog" | ||
log "github.com/sirupsen/logrus" | ||
"golang.org/x/net/context" | ||
) | ||
|
@@ -185,20 +184,46 @@ func (txn *tikvTxn) Commit(ctx context.Context) error { | |
connID = val.(uint64) | ||
} | ||
committer, err := newTwoPhaseCommitter(txn, connID) | ||
if err != nil { | ||
if err != nil || committer == 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. When will 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. When no row needs to update in this transaction. |
||
return errors.Trace(err) | ||
} | ||
if committer == nil { | ||
return nil | ||
|
||
defer func() { | ||
if 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. I guess this can be moved into |
||
txn.commitTS = committer.commitTS | ||
} | ||
}() | ||
// latches disabled | ||
if txn.store.txnLatches == nil { | ||
err = committer.executeAndWriteFinishBinlog(ctx) | ||
log.Debug("[kv]", connID, " txnLatches disabled, 2pc directly:", err) | ||
return errors.Trace(err) | ||
} | ||
err = committer.execute(ctx) | ||
if err != nil { | ||
committer.writeFinishBinlog(binlog.BinlogType_Rollback, 0) | ||
|
||
// latches enabled | ||
// for transactions not retryable, commit directly. | ||
if !sessionctx.GetRetryable(ctx) { | ||
err = committer.executeAndWriteFinishBinlog(ctx) | ||
if err == nil { | ||
txn.store.txnLatches.RefreshCommitTS(committer.keys, committer.commitTS) | ||
} | ||
log.Debug("[kv]", connID, " txnLatches enabled while txn not retryable, 2pc directly:", err) | ||
return errors.Trace(err) | ||
} | ||
committer.writeFinishBinlog(binlog.BinlogType_Commit, int64(committer.commitTS)) | ||
txn.commitTS = committer.commitTS | ||
return nil | ||
|
||
// for transactions which need to acquire latches | ||
lock := txn.store.txnLatches.Lock(committer.startTS, committer.keys) | ||
defer txn.store.txnLatches.UnLock(lock) | ||
if lock.IsStale() { | ||
err = errors.Errorf("startTS %d is stale", txn.startTS) | ||
return errors.Annotate(err, txnRetryableMark) | ||
} | ||
err = committer.executeAndWriteFinishBinlog(ctx) | ||
if err == nil { | ||
lock.SetCommitTS(committer.commitTS) | ||
} | ||
log.Debug("[kv]", connID, " txnLatches enabled while txn retryable:", err) | ||
return errors.Trace(err) | ||
} | ||
|
||
func (txn *tikvTxn) close() { | ||
|
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.
I really dislike passing those value through context, even
txn.SetOption(retryable)
is better than this.If you insist, I'll refactor the code here after it's merged.
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.
I've tried your suggestion and it's a little complex. I think to refactor the code in another PR is
ok for me.