support distributed dead lock detection #263
Conversation
|
@coocood PTAL distributed tests not done yet, thx |
cfzjywxk
changed the title
tikv/deadlock.go
Outdated
| log.Errorf("refresh leader addr failed, err=%v", refreshErr) | ||
| } | ||
| time.Sleep(3 * time.Second) | ||
| continue |
There was a problem hiding this comment.
The failed req will be discarded when retry?
There was a problem hiding this comment.
Yes, the request will be timeout and the pessimistic lock error will be reported instead of possible deadlock error, tidb may retry the pessimistic lock request
tikv/deadlock.go
Outdated
| } | ||
|
|
||
| // Detect will process the detection request on local detector | ||
| func (dt *DeadlockDetector) Detect(txnTs uint64, waitForTxnTs uint64, keyHash uint64) error { |
There was a problem hiding this comment.
Remove the local detector optimization would make the code much simpler.
There was a problem hiding this comment.
This is used mainly for Server.Detect RPC service handler
There was a problem hiding this comment.
We can separate to DetectorClient and DetectorServer, DetectorClient doesn't need to contain the Detector type.
There was a problem hiding this comment.
DeadlockDetector ->(split into) DetectorClient and DetectorServer , and
DetectorServer wraps the detector type and do real local detection(used by rpc handler),
DetectorClient warps connection related things and is responcible for send and recv
do I understand right? Seems same code lines 😅
There was a problem hiding this comment.
@coocood only detection client interface kept, others removed
|
LGTM |
Issue #257
two related changes could be reviewed separately #269 #270
Support distributed deadlock detection
deadlockproto related RPC server interfacedetectorinto newDeadLockDetector, add role to this new detector, and process requests based on current role(leader/follower)detectConnanddetectClientto do streamsendandrecv[this wrapping removed]OnRoleChangetikv.detector.go, and add some extra utilities.The storage engine still has some problems #267 #268, the distributed tests are not done yet.