-
Notifications
You must be signed in to change notification settings - Fork 217
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
lock_resolver: support verifying primary for check_txn_status #777
lock_resolver: support verifying primary for check_txn_status #777
Conversation
Signed-off-by: MyonKeminta <[email protected]>
Signed-off-by: MyonKeminta <[email protected]>
This PR is now reviewable. The integration test failed because the TiKV image is not updated yet and doesn't include the latest change. It should pass tomorrow. |
Signed-off-by: MyonKeminta <[email protected]>
Signed-off-by: MyonKeminta <[email protected]>
Signed-off-by: MyonKeminta <[email protected]>
txnkv/txnlock/lock_resolver.go
Outdated
@@ -448,7 +448,11 @@ func (lr *LockResolver) resolveLocks(bo *retry.Backoffer, opts ResolveLocksOptio | |||
var resolve func(*Lock, bool) (TxnStatus, error) | |||
resolve = func(l *Lock, forceSyncCommit bool) (TxnStatus, error) { | |||
status, err := lr.getTxnStatusFromLock(bo, l, callerStartTS, forceSyncCommit, detail) | |||
if err != nil { | |||
|
|||
if _, ok := errors.Cause(err).(primaryMismatch); ok && l.LockType == kvrpcpb.Op_PessimisticLock { |
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.
Maybe we could print an ERROR
or FATAL
log for the l.LockType != kvrpcpb.Op_PessimisticLock
path since it's unexpected?
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'll add an error here. Actually the error will be returned to TiDB, and TiDB will print it as an unknown error anyway.
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 noticed that there might be other problem about the primary when fair-locking is used...
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.
@MyonKeminta
The check issue for fair lock is recorded in pingcap/tidb#43540, could we merge this 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.
@MyonKeminta The check issue for fair lock is recorded in pingcap/tidb#43540, could we merge this first?
yes. lets merge it.
Signed-off-by: MyonKeminta <[email protected]>
Co-authored-by: MyonKeminta <[email protected]> Co-authored-by: disksing <[email protected]> Co-authored-by: MyonKeminta <[email protected]> Co-authored-by: Violin <[email protected]> Co-authored-by: Smilencer <[email protected]> Co-authored-by: you06 <[email protected]> Co-authored-by: Hu# <[email protected]> Co-authored-by: Connor <[email protected]> Co-authored-by: zyguan <[email protected]> fix case typo in comment. (#778) fix goroutine leak (#784) fix TestRURuntimeStatsCleanUp (#787) Fix wrong resource group name for some requests (#788) resolver: support verifying primary for check_txn_status (#777) resolver: handle pessimistic locks in BatchResolveLocks (#794) resolved ts (#793) ResolveLocks for unistore (#807)
* support verifying primary for check_txn_status Signed-off-by: MyonKeminta <[email protected]> * update kvproto Signed-off-by: MyonKeminta <[email protected]> * add more failpoint usages Signed-off-by: MyonKeminta <[email protected]> * update depencency and fix test Signed-off-by: MyonKeminta <[email protected]> * Do not skip for unistore; refine logs Signed-off-by: MyonKeminta <[email protected]> * Address comments Signed-off-by: MyonKeminta <[email protected]> --------- Signed-off-by: MyonKeminta <[email protected]> Co-authored-by: MyonKeminta <[email protected]>
…777) (#819) * Update dependencies Signed-off-by: MyonKeminta <[email protected]> * lock_resolver: support verifying primary for check_txn_status (#777) * support verifying primary for check_txn_status Signed-off-by: MyonKeminta <[email protected]> * update kvproto Signed-off-by: MyonKeminta <[email protected]> * add more failpoint usages Signed-off-by: MyonKeminta <[email protected]> * update depencency and fix test Signed-off-by: MyonKeminta <[email protected]> * Do not skip for unistore; refine logs Signed-off-by: MyonKeminta <[email protected]> * Address comments Signed-off-by: MyonKeminta <[email protected]> --------- Signed-off-by: MyonKeminta <[email protected]> Co-authored-by: MyonKeminta <[email protected]> * Try to trigger unit tests Signed-off-by: MyonKeminta <[email protected]> * update dependency Signed-off-by: MyonKeminta <[email protected]> * Revert "Try to trigger unit tests" This reverts commit 7a6ed8d. Signed-off-by: MyonKeminta <[email protected]> * trigger ci Signed-off-by: MyonKeminta <[email protected]> --------- Signed-off-by: MyonKeminta <[email protected]> Co-authored-by: MyonKeminta <[email protected]>
Ref: pingcap/tidb#42937
Requires: pingcap/kvproto#1105
Requires: tikv/tikv#14637