Skip to content

Commit

Permalink
address comments
Browse files Browse the repository at this point in the history
Signed-off-by: Ryan Leung <rleungx@gmail.com>
  • Loading branch information
rleungx committed May 12, 2023
1 parent e1af99b commit 343bda6
Show file tree
Hide file tree
Showing 5 changed files with 36 additions and 22 deletions.
2 changes: 1 addition & 1 deletion pkg/mcs/resource_manager/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,7 @@ func (s *Server) initClient() error {
if err != nil {
return err
}
s.etcdClient, _, s.httpClient, err = etcdutil.CreateClients(tlsConfig, []url.URL(u)[0])
s.etcdClient, s.httpClient, err = etcdutil.CreateClients(tlsConfig, []url.URL(u)[0])
return err
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/mcs/tso/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,7 @@ func (s *Server) initClient() error {
if err != nil {
return err
}
s.etcdClient, _, s.httpClient, err = etcdutil.CreateClients(tlsConfig, s.backendUrls[0])
s.etcdClient, s.httpClient, err = etcdutil.CreateClients(tlsConfig, s.backendUrls[0])
return err
}

Expand Down
16 changes: 6 additions & 10 deletions pkg/utils/etcdutil/etcdutil.go
Original file line number Diff line number Diff line change
Expand Up @@ -197,17 +197,13 @@ func EtcdKVPutWithTTL(ctx context.Context, c *clientv3.Client, key string, value
}

// CreateClients creates etcd v3 client and http client.
func CreateClients(tlsConfig *tls.Config, acUrls url.URL) (*clientv3.Client, *clientv3.Client, *http.Client, error) {
client, err := createEtcdClient(tlsConfig, acUrls)
func CreateClients(tlsConfig *tls.Config, acUrls url.URL) (*clientv3.Client, *http.Client, error) {
client, err := CreateEtcdClient(tlsConfig, acUrls)
if err != nil {
return nil, nil, nil, errs.ErrNewEtcdClient.Wrap(err).GenWithStackByCause()
}
electionClient, err := createEtcdClient(tlsConfig, acUrls)
if err != nil {
return nil, nil, nil, errs.ErrNewEtcdClient.Wrap(err).GenWithStackByCause()
return nil, nil, errs.ErrNewEtcdClient.Wrap(err).GenWithStackByCause()
}
httpClient := createHTTPClient(tlsConfig)
return electionClient, client, httpClient, nil
return client, httpClient, nil
}

// createEtcdClientWithMultiEndpoint creates etcd v3 client.
Expand Down Expand Up @@ -249,9 +245,9 @@ func createEtcdClientWithMultiEndpoint(tlsConfig *tls.Config, acUrls []url.URL)
return client, err
}

// createEtcdClient creates etcd v3 client.
// CreateEtcdClient creates etcd v3 client.
// Note: it will be used by legacy pd-server, and only connect to leader only.
func createEtcdClient(tlsConfig *tls.Config, acURL url.URL) (*clientv3.Client, error) {
func CreateEtcdClient(tlsConfig *tls.Config, acURL url.URL) (*clientv3.Client, error) {
lgc := zap.NewProductionConfig()
lgc.Encoding = log.ZapEncodingName
client, err := clientv3.New(clientv3.Config{
Expand Down
12 changes: 6 additions & 6 deletions pkg/utils/etcdutil/etcdutil_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -305,12 +305,12 @@ func TestEtcdScaleInAndOutWithoutMultiPoint(t *testing.T) {
// Create two etcd clients with etcd1 as endpoint.
urls, err := types.NewURLs([]string{ep1})
re.NoError(err)
client1, err := createEtcdClient(nil, urls[0]) // execute member change operation with this client
client1, err := CreateEtcdClient(nil, urls[0]) // execute member change operation with this client
defer func() {
client1.Close()
}()
re.NoError(err)
client2, err := createEtcdClient(nil, urls[0]) // check member change with this client
client2, err := CreateEtcdClient(nil, urls[0]) // check member change with this client
defer func() {
client2.Close()
}()
Expand Down Expand Up @@ -482,7 +482,7 @@ func (suite *loopWatcherTestSuite) SetupSuite() {
ep1 := suite.config.LCUrls[0].String()
urls, err := types.NewURLs([]string{ep1})
suite.NoError(err)
suite.client, err = createEtcdClient(nil, urls[0])
suite.client, err = CreateEtcdClient(nil, urls[0])
suite.NoError(err)
suite.cleans = append(suite.cleans, func() {
suite.client.Close()
Expand Down Expand Up @@ -685,23 +685,23 @@ func (suite *loopWatcherTestSuite) TestWatcherBreak() {

// Case2: close the etcd client and put a new value after watcher restarts
suite.client.Close()
suite.client, err = createEtcdClient(nil, suite.config.LCUrls[0])
suite.client, err = CreateEtcdClient(nil, suite.config.LCUrls[0])
suite.NoError(err)
watcher.updateClientCh <- suite.client
suite.put("TestWatcherBreak", "2")
checkCache("2")

// Case3: close the etcd client and put a new value before watcher restarts
suite.client.Close()
suite.client, err = createEtcdClient(nil, suite.config.LCUrls[0])
suite.client, err = CreateEtcdClient(nil, suite.config.LCUrls[0])
suite.NoError(err)
suite.put("TestWatcherBreak", "3")
watcher.updateClientCh <- suite.client
checkCache("3")

// Case4: close the etcd client and put a new value with compact
suite.client.Close()
suite.client, err = createEtcdClient(nil, suite.config.LCUrls[0])
suite.client, err = CreateEtcdClient(nil, suite.config.LCUrls[0])
suite.NoError(err)
suite.put("TestWatcherBreak", "4")
resp, err := EtcdKVGet(suite.client, "TestWatcherBreak")
Expand Down
26 changes: 22 additions & 4 deletions server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,12 @@ func (s *Server) startEtcd(ctx context.Context) error {
}

// start client
s.client, s.electionClient, s.httpClient, err = startClient(s.cfg)
s.client, s.httpClient, err = startClient(s.cfg)
if err != nil {
return err
}

s.electionClient, err = startElectionClient(s.cfg)
if err != nil {
return err
}
Expand All @@ -359,18 +364,31 @@ func (s *Server) startEtcd(ctx context.Context) error {
return nil
}

func startClient(cfg *config.Config) (*clientv3.Client, *clientv3.Client, *http.Client, error) {
func startClient(cfg *config.Config) (*clientv3.Client, *http.Client, error) {
tlsConfig, err := cfg.Security.ToTLSConfig()
if err != nil {
return nil, nil, nil, err
return nil, nil, err
}
etcdCfg, err := cfg.GenEmbedEtcdConfig()
if err != nil {
return nil, nil, nil, err
return nil, nil, err
}
return etcdutil.CreateClients(tlsConfig, etcdCfg.ACUrls[0])
}

func startElectionClient(cfg *config.Config) (*clientv3.Client, error) {
tlsConfig, err := cfg.Security.ToTLSConfig()
if err != nil {
return nil, err
}
etcdCfg, err := cfg.GenEmbedEtcdConfig()
if err != nil {
return nil, err
}

return etcdutil.CreateEtcdClient(tlsConfig, etcdCfg.ACUrls[0])
}

// AddStartCallback adds a callback in the startServer phase.
func (s *Server) AddStartCallback(callbacks ...func()) {
s.startCallbacks = append(s.startCallbacks, callbacks...)
Expand Down

0 comments on commit 343bda6

Please sign in to comment.