server: refactor the independent service check #8738
Conversation
Signed-off-by: Ryan Leung <[email protected]>
ti-chi-bot
bot
added
release-note-none
Signed-off-by: Ryan Leung <[email protected]>
Signed-off-by: Ryan Leung <[email protected]>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #8738 +/- ##
==========================================
+ Coverage 70.10% 75.66% +5.55%
==========================================
Files 517 460 -57
Lines 79983 72107 -7876
==========================================
- Hits 56076 54557 -1519
+ Misses 20339 14056 -6283
+ Partials 3568 3494 -74
Flags with carried forward coverage won't be shown. Click here to find out more. |
| if name == constant.TSOServiceName { | ||
| return true | ||
| } | ||
| return s.cluster.IsServiceIndependent(name) |
There was a problem hiding this comment.
Do we need to check whether cluster is nil?
| forwardedHost, ok := s.GetServicePrimaryAddr(stream.Context(), constant.TSOServiceName) | ||
| if !ok || len(forwardedHost) == 0 { | ||
| tsoStreamErr = errors.WithStack(ErrNotFoundTSOAddr) | ||
| forwardCtx, cancelForward, forwardStream, lastForwardedHost, tsoStreamErr, err = s.handleTSOForwarding(forwardCtx, forwardStream, stream, server, request, tsDeadlineCh, lastForwardedHost, cancelForward) |
There was a problem hiding this comment.
There are so many incoming and returning parameters here
There was a problem hiding this comment.
Yes. But I don't find a better way to avoid it.
There was a problem hiding this comment.
Is there any benefit after wraping handleTSOForwarding?
| Count: tsopbResp.GetCount(), | ||
| Timestamp: tsopbResp.GetTimestamp(), | ||
| } | ||
| if server != nil { |
There was a problem hiding this comment.
We receive both stream pdpb.PD_TsoServer and server *tsoServer as the parameters.
There was a problem hiding this comment.
Server seems that will not be nil?🤔
ti-chi-bot
bot
added
needs-1-more-lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: lhy1024, okJiang The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
ti-chi-bot
bot
added
lgtm
and removed
needs-1-more-lgtm
|
/retest |
What problem does this PR solve?
Issue Number: ref #8477.
What is changed and how does it work?
This is a part of #8517.
Check List
Tests
Release note