Skip to content
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

http: init NewDefaultPDServiceDiscovery and refine http client tests #7744

Merged
merged 6 commits into from
Jan 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 23 additions & 16 deletions client/http/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
"encoding/json"
"io"
"net/http"
"os"
"time"

"github.com/pingcap/errors"
Expand Down Expand Up @@ -270,21 +269,6 @@
}
}

// WithLoggerRedirection configures the client with the given logger redirection.
func WithLoggerRedirection(logLevel, fileName string) ClientOption {
cfg := &log.Config{}
cfg.Level = logLevel
if fileName != "" {
f, _ := os.CreateTemp(".", fileName)
fname := f.Name()
f.Close()
cfg.File.Filename = fname
}
lg, p, _ := log.InitLogger(cfg)
log.ReplaceGlobals(lg, p)
return func(c *client) {}
}

// NewClientWithServiceDiscovery creates a PD HTTP client with the given PD service discovery.
func NewClientWithServiceDiscovery(
source string,
Expand Down Expand Up @@ -314,6 +298,10 @@
opt(c)
}
sd := pd.NewDefaultPDServiceDiscovery(ctx, cancel, pdAddrs, c.inner.tlsConf)
if err := sd.Init(); err != nil {
log.Error("[pd] init service discovery failed", zap.String("source", source), zap.Strings("pd-addrs", pdAddrs), zap.Error(err))
return nil

Check warning on line 303 in client/http/client.go

View check run for this annotation

Codecov / codecov/patch

client/http/client.go#L302-L303

Added lines #L302 - L303 were not covered by tests
}
c.inner.init(sd)
return c
}
Expand Down Expand Up @@ -371,6 +359,7 @@
headerOpts...)
}

/* The following functions are only for test */
// requestChecker is used to check the HTTP request sent by the client.
type requestChecker func(req *http.Request) error

Expand All @@ -385,3 +374,21 @@
Transport: checker,
}
}

// newClientWithoutInitServiceDiscovery creates a PD HTTP client
// with the given PD addresses and TLS config without init service discovery.
func newClientWithoutInitServiceDiscovery(
source string,
pdAddrs []string,
opts ...ClientOption,
) Client {
ctx, cancel := context.WithCancel(context.Background())
c := &client{inner: newClientInner(ctx, cancel, source), callerID: defaultCallerID}
// Apply the options first.
for _, opt := range opts {
opt(c)
}
sd := pd.NewDefaultPDServiceDiscovery(ctx, cancel, pdAddrs, c.inner.tlsConf)
c.inner.init(sd)
return c
}
6 changes: 3 additions & 3 deletions client/http/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ func TestPDAllowFollowerHandleHeader(t *testing.T) {
}
return nil
})
c := NewClient("test-header", []string{"http://127.0.0.1"}, WithHTTPClient(httpClient))
c := newClientWithoutInitServiceDiscovery("test-header", []string{"http://127.0.0.1"}, WithHTTPClient(httpClient))
c.GetRegions(context.Background())
c.GetHistoryHotRegions(context.Background(), &HistoryHotRegionsRequest{})
c.Close()
Expand All @@ -58,7 +58,7 @@ func TestCallerID(t *testing.T) {
}
return nil
})
c := NewClient("test-caller-id", []string{"http://127.0.0.1"}, WithHTTPClient(httpClient))
c := newClientWithoutInitServiceDiscovery("test-caller-id", []string{"http://127.0.0.1"}, WithHTTPClient(httpClient))
c.GetRegions(context.Background())
expectedVal.Store("test")
c.WithCallerID(expectedVal.Load()).GetRegions(context.Background())
Expand All @@ -69,7 +69,7 @@ func TestWithBackoffer(t *testing.T) {
re := require.New(t)
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
c := NewClient("test-with-backoffer", []string{"http://127.0.0.1"})
c := newClientWithoutInitServiceDiscovery("test-with-backoffer", []string{"http://127.0.0.1"})

base := 100 * time.Millisecond
max := 500 * time.Millisecond
Expand Down
4 changes: 3 additions & 1 deletion client/pd_service_discovery.go
Original file line number Diff line number Diff line change
Expand Up @@ -911,7 +911,9 @@ func (c *pdServiceDiscovery) checkServiceModeChanged() error {
if clusterInfo == nil || len(clusterInfo.ServiceModes) == 0 {
return errors.WithStack(errNoServiceModeReturned)
}
c.serviceModeUpdateCb(clusterInfo.ServiceModes[0])
if c.serviceModeUpdateCb != nil {
c.serviceModeUpdateCb(clusterInfo.ServiceModes[0])
}
return nil
}

Expand Down
Loading
Loading