store/tikv: reduce the lock contend between sending message and re-creating streaming client #11301
Conversation
| @@ -224,6 +248,7 @@ func (c *batchCommandsClient) reCreateStreamingClient(err error) bool { | |||
| zap.String("target", c.target), | |||
| ) | |||
| c.client = streamClient | |||
There was a problem hiding this comment.
this write becomes a write without hold "mutex", can other thread saw this change?
There was a problem hiding this comment.
reCreateStreamingClient acquire the write lock, so other send goroutine skips this batchCommandsClient and try the next one, in tryLockForSend @lysu
There was a problem hiding this comment.
This function is protected under lock already
There was a problem hiding this comment.
lockForRecreate set a flag to exclusive others to step into this, but it call mutex.Unlock after set flag, so this function can exclusive other access, but I still confuse its visibility 😕
There was a problem hiding this comment.
After lockForRecreate, the reCreate flag is set. tryLockForSend will detect the flag.
Maybe you can implement a RWMutex struct with a tryRlock API, and then you'll figure out it.
|
/run-all-tests |
Codecov Report
@@ Coverage Diff @@
## master #11301 +/- ##
==============================================
- Coverage 81.94% 81.2788% -0.6612%
==============================================
Files 424 423 -1
Lines 92763 90256 -2507
==============================================
- Hits 76010 73359 -2651
- Misses 11473 11598 +125
- Partials 5280 5299 +19 |
|
/run-all-tests |
|
rest LGTM. |
|
cherry pick to release-3.0 failed |
…eating streaming client (pingcap#11301)
What problem does this PR solve?
Tiny code refactor
What is changed and how it works?
We use a lock here
https://github.com/pingcap/tidb/compare/master...tiancaiamao:batch-client-lock?expand=1#diff-db62a1f8315185d693e2a911ce0d5b49L176
and here
https://github.com/pingcap/tidb/compare/master...tiancaiamao:batch-client-lock?expand=1#diff-db62a1f8315185d693e2a911ce0d5b49L214
Two write-write locks to protect the send and re-create operation.
The caller of the send operation is from the
batchSendLoop, that means the whole send loop will be blocked during the streaming client re-creating.In this commit, I provide a
tryLockForSendfunction to try the lock first, if thebatchCommandsClientis re-creating, we can try another one, instead of blocking thebatchSendLoop.Check List
Tests
Related changes