client: print more info when leader disconnect #7907
Conversation
Signed-off-by: Cabinfever_B <[email protected]>
|
[REVIEW NOTIFICATION] This pull request has not been approved. To complete the pull request process, please ask the reviewers in the list to review by filling The full list of commands accepted by this bot can be found here. Reviewer can indicate their review by submitting an approval review. |
ti-chi-bot
bot
added
the
release-note-none
CabinfeverB
requested review from
HuSharp
and removed request for
JmPotato and
rleungx
ti-chi-bot
bot
added
the
size/M
| leaderClient := newPDServiceClient(addr, addr, c.tlsCfg, newConn, true) | ||
| c.leader.Store(leaderClient) | ||
| if addr != oldLeader.GetAddress() { | ||
| change = true |
There was a problem hiding this comment.
It might fail to connect to the leader?
There was a problem hiding this comment.
Yes, but the leader does change and it will affect the follower proxy
Signed-off-by: Cabinfever_B <[email protected]>
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #7907 +/- ##
==========================================
+ Coverage 73.32% 73.60% +0.28%
==========================================
Files 435 436 +1
Lines 48195 48349 +154
==========================================
+ Hits 35337 35586 +249
+ Misses 9784 9718 -66
+ Partials 3074 3045 -29
Flags with carried forward coverage won't be shown. Click here to find out more. |
Signed-off-by: Cabinfever_B <[email protected]>
| } else { | ||
| log.Warn("[pd] failed to connect leader", zap.String("leader", url), errs.ZapError(err)) | ||
| } | ||
| if change { | ||
| // Set PD leader and Global TSO Allocator (which is also the PD leader) | ||
| leaderClient := newPDServiceClient(url, url, newConn, true) |
There was a problem hiding this comment.
Does newConn will be nill? Do we need to change if change to if change || newConn != nil ?
There was a problem hiding this comment.
Even if the connection is not established, the leader is still updated.
The reason is that the leader has transferred to another PD server, and the client should update the member information. Meanwhile, the client must know the newest leader for follower proxy.
Signed-off-by: Cabinfever_B <[email protected]>
client/pd_service_discovery.go
Outdated
| return true, err | ||
| // Run callbacks | ||
| if c.tsoGlobalAllocLeaderUpdatedCb != nil { | ||
| err = multierr.Append(err, c.tsoGlobalAllocLeaderUpdatedCb(url)) |
There was a problem hiding this comment.
I am not sure if it is necessary to return both connection establishment and call back errors
Signed-off-by: Cabinfever_B <[email protected]>
Signed-off-by: Cabinfever_B <[email protected]>
CabinfeverB
changed the title
CabinfeverB
changed the title
Signed-off-by: Cabinfever_B <[email protected]>
Signed-off-by: Cabinfever_B <[email protected]>
| if url != oldLeader.GetURL() { | ||
| change = true |
There was a problem hiding this comment.
This check and introducing change seem unnecessary due to line 999.
There was a problem hiding this comment.
If the client continues to fail to establish a connection with the leader, there will be situations where oldURL == url and err != nil``. So it's not always change = true` after line 999
What problem does this PR solve?
Issue Number: Close #7906
should be merged after #7911
What is changed and how does it work?
Check List
Tests
Code changes
Side effects
Related changes
pingcap/docs/pingcap/docs-cn:pingcap/tiup:Release note