From b8ce9c05b866daa745dd0f5f90d5a3107d7f39ae Mon Sep 17 00:00:00 2001 From: djshow832 Date: Mon, 13 Nov 2023 10:33:43 +0800 Subject: [PATCH] backend, infosync: optimize `TestHandlerReturnError` and `TestEtcdServerDown4Sync` (#398) --- pkg/manager/infosync/info_test.go | 26 ++++++++++------------ pkg/proxy/backend/authenticator.go | 5 ++--- pkg/proxy/backend/backend_conn_mgr.go | 14 +++++++++--- pkg/proxy/backend/backend_conn_mgr_test.go | 6 ++--- pkg/proxy/backend/mock_proxy_test.go | 3 +-- 5 files changed, 29 insertions(+), 25 deletions(-) diff --git a/pkg/manager/infosync/info_test.go b/pkg/manager/infosync/info_test.go index 2983c4d6..1573e897 100644 --- a/pkg/manager/infosync/info_test.go +++ b/pkg/manager/infosync/info_test.go @@ -54,20 +54,18 @@ func TestEtcdServerDown4Sync(t *testing.T) { ts := newEtcdTestSuite(t) t.Cleanup(ts.close) var ttl string - for i := 0; i < 5; i++ { - // Make the server down for some time. - addr := ts.shutdownServer() - time.Sleep(time.Second) - ts.startServer(addr) - require.Eventually(t, func() bool { - newTTL, info := ts.getTTLAndInfo(tiproxyTopologyPath) - satisfied := newTTL != ttl && len(info) > 0 - if satisfied { - ttl = newTTL - } - return satisfied - }, 5*time.Second, 100*time.Millisecond) - } + // Make the server down for some time. + addr := ts.shutdownServer() + time.Sleep(time.Second) + ts.startServer(addr) + require.Eventually(t, func() bool { + newTTL, info := ts.getTTLAndInfo(tiproxyTopologyPath) + satisfied := newTTL != ttl && len(info) > 0 + if satisfied { + ttl = newTTL + } + return satisfied + }, 5*time.Second, 100*time.Millisecond) } // TTL and info are erased after the client shuts down normally. diff --git a/pkg/proxy/backend/authenticator.go b/pkg/proxy/backend/authenticator.go index 6b035f8f..f5acb6e2 100644 --- a/pkg/proxy/backend/authenticator.go +++ b/pkg/proxy/backend/authenticator.go @@ -9,7 +9,6 @@ import ( "encoding/binary" "fmt" "net" - "time" "github.com/go-mysql-org/go-mysql/mysql" "github.com/pingcap/tidb/util/hack" @@ -85,7 +84,7 @@ func (auth *Authenticator) verifyBackendCaps(logger *zap.Logger, backendCapabili return nil } -type backendIOGetter func(ctx ConnContext, auth *Authenticator, resp *pnet.HandshakeResp, timeout time.Duration) (*pnet.PacketIO, error) +type backendIOGetter func(ctx ConnContext, auth *Authenticator, resp *pnet.HandshakeResp) (*pnet.PacketIO, error) func (auth *Authenticator) handshakeFirstTime(logger *zap.Logger, cctx ConnContext, clientIO *pnet.PacketIO, handshakeHandler HandshakeHandler, getBackendIO backendIOGetter, frontendTLSConfig, backendTLSConfig *tls.Config) error { @@ -162,7 +161,7 @@ func (auth *Authenticator) handshakeFirstTime(logger *zap.Logger, cctx ConnConte RECONNECT: // In case of testing, backendIO is passed manually that we don't want to bother with the routing logic. - backendIO, err := getBackendIO(cctx, auth, clientResp, 15*time.Second) + backendIO, err := getBackendIO(cctx, auth, clientResp) if err != nil { return pnet.WrapUserError(err, connectErrMsg) } diff --git a/pkg/proxy/backend/backend_conn_mgr.go b/pkg/proxy/backend/backend_conn_mgr.go index bfe9c58c..5123734d 100644 --- a/pkg/proxy/backend/backend_conn_mgr.go +++ b/pkg/proxy/backend/backend_conn_mgr.go @@ -33,7 +33,11 @@ var ( ) const ( - DialTimeout = 1 * time.Second + // DialTimeout is the timeout for each dial. + DialTimeout = 1 * time.Second + // ConnectTimeout is the timeout for choosing and connecting to an available backend. + ConnectTimeout = 15 * time.Second + // CheckBackendInterval is the interval for checking if the backend is still connected. CheckBackendInterval = time.Minute ) @@ -74,6 +78,7 @@ type BCConfig struct { HealthyKeepAlive config.KeepAlive UnhealthyKeepAlive config.KeepAlive CheckBackendInterval time.Duration + ConnectTimeout time.Duration ConnBufferSize int ProxyProtocol bool RequireBackendTLS bool @@ -83,6 +88,9 @@ func (cfg *BCConfig) check() { if cfg.CheckBackendInterval == time.Duration(0) { cfg.CheckBackendInterval = CheckBackendInterval } + if cfg.ConnectTimeout == time.Duration(0) { + cfg.ConnectTimeout = ConnectTimeout + } } // BackendConnManager migrates a session from one BackendConnection to another. @@ -180,7 +188,7 @@ func (mgr *BackendConnManager) Connect(ctx context.Context, clientIO *pnet.Packe return nil } -func (mgr *BackendConnManager) getBackendIO(cctx ConnContext, auth *Authenticator, resp *pnet.HandshakeResp, timeout time.Duration) (*pnet.PacketIO, error) { +func (mgr *BackendConnManager) getBackendIO(cctx ConnContext, auth *Authenticator, resp *pnet.HandshakeResp) (*pnet.PacketIO, error) { r, err := mgr.handshakeHandler.GetRouter(cctx, resp) if err != nil { return nil, pnet.WrapUserError(err, err.Error()) @@ -188,7 +196,7 @@ func (mgr *BackendConnManager) getBackendIO(cctx ConnContext, auth *Authenticato // Reasons to wait: // - The TiDB instances may not be initialized yet // - One TiDB may be just shut down and another is just started but not ready yet - bctx, cancel := context.WithTimeout(context.Background(), timeout) + bctx, cancel := context.WithTimeout(context.Background(), mgr.config.ConnectTimeout) selector := r.GetBackendSelector() startTime := time.Now() var addr string diff --git a/pkg/proxy/backend/backend_conn_mgr_test.go b/pkg/proxy/backend/backend_conn_mgr_test.go index 5c6173a1..f07f3329 100644 --- a/pkg/proxy/backend/backend_conn_mgr_test.go +++ b/pkg/proxy/backend/backend_conn_mgr_test.go @@ -761,8 +761,8 @@ func TestHandlerReturnError(t *testing.T) { quitSource: SrcProxyErr, }, { - // TODO: make it fail faster. cfg: func(config *testConfig) { + config.proxyConfig.bcConfig.ConnectTimeout = time.Second config.proxyConfig.handler.getRouter = func(ctx ConnContext, resp *pnet.HandshakeResp) (router.Router, error) { return router.NewStaticRouter(nil), nil } @@ -857,7 +857,7 @@ func TestGetBackendIO(t *testing.T) { }, } lg, _ := logger.CreateLoggerForTest(t) - mgr := NewBackendConnManager(lg, handler, 0, &BCConfig{}) + mgr := NewBackendConnManager(lg, handler, 0, &BCConfig{ConnectTimeout: time.Second}) var wg waitgroup.WaitGroup for i := 0; i <= len(listeners); i++ { wg.Run(func() { @@ -867,7 +867,7 @@ func TestGetBackendIO(t *testing.T) { require.NoError(t, cn.Close()) } }) - io, err := mgr.getBackendIO(mgr, mgr.authenticator, nil, time.Second) + io, err := mgr.getBackendIO(mgr, mgr.authenticator, nil) if err == nil { require.NoError(t, io.Close()) } diff --git a/pkg/proxy/backend/mock_proxy_test.go b/pkg/proxy/backend/mock_proxy_test.go index c6c1051e..ee3fad57 100644 --- a/pkg/proxy/backend/mock_proxy_test.go +++ b/pkg/proxy/backend/mock_proxy_test.go @@ -6,7 +6,6 @@ package backend import ( "crypto/tls" "testing" - "time" gomysql "github.com/go-mysql-org/go-mysql/mysql" "github.com/pingcap/tiproxy/lib/util/logger" @@ -58,7 +57,7 @@ func newMockProxy(t *testing.T, cfg *proxyConfig) *mockProxy { } func (mp *mockProxy) authenticateFirstTime(clientIO, backendIO *pnet.PacketIO) error { - if err := mp.authenticator.handshakeFirstTime(mp.logger, mp, clientIO, mp.handshakeHandler, func(ctx ConnContext, auth *Authenticator, resp *pnet.HandshakeResp, timeout time.Duration) (*pnet.PacketIO, error) { + if err := mp.authenticator.handshakeFirstTime(mp.logger, mp, clientIO, mp.handshakeHandler, func(ctx ConnContext, auth *Authenticator, resp *pnet.HandshakeResp) (*pnet.PacketIO, error) { return backendIO, nil }, mp.frontendTLSConfig, mp.backendTLSConfig); err != nil { return err