From 3c7d6d956419c42afa6f2dc71ed54bdc0ab6ac29 Mon Sep 17 00:00:00 2001 From: richardjcai Date: Mon, 11 Jul 2022 15:59:01 -0400 Subject: [PATCH] sql: fix flakiness with InternalExecutorFactoryMemMonitor closing TestServers We do not actually have to worry about closing the InternalExecutorFactoryMemMonitor. Release note: None --- pkg/ccl/sqlproxyccl/backend_dialer_test.go | 3 --- pkg/server/server_sql.go | 8 ++++++-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/pkg/ccl/sqlproxyccl/backend_dialer_test.go b/pkg/ccl/sqlproxyccl/backend_dialer_test.go index c537a045effc..699e562cfc76 100644 --- a/pkg/ccl/sqlproxyccl/backend_dialer_test.go +++ b/pkg/ccl/sqlproxyccl/backend_dialer_test.go @@ -44,9 +44,6 @@ func TestBackendDialTLS(t *testing.T) { defer sql.Stopper().Stop(ctx) conn, err := BackendDial(startupMsg, sql.ServingSQLAddr(), tlsConfig) - defer func() { - _ = conn.Close() - }() require.NoError(t, err) require.NotNil(t, conn) diff --git a/pkg/server/server_sql.go b/pkg/server/server_sql.go index 303eb35563f5..3d798e1a2444 100644 --- a/pkg/server/server_sql.go +++ b/pkg/server/server_sql.go @@ -878,7 +878,12 @@ func newSQLServer(ctx context.Context, cfg sqlServerArgs) (*SQLServer, error) { distSQLServer.ServerConfig.IndexUsageStatsController = pgServer.SQLServer.GetIndexUsageStatsController() // We use one BytesMonitor for all InternalExecutor's created by the - // ieFactory, this BytesMonitor is never closed. + // ieFactory. + // Note that ieFactoryMonitor does not have to be closed, the parent + // monitor comes from server. ieFactoryMonitor is a singleton attached + // to server, if server is closed, we don't have to worry about + // returning the memory allocated to ieFactoryMonitor since the + // parent monitor is being closed anyway. ieFactoryMonitor := mon.NewMonitor( "internal executor factory", mon.MemoryResource, @@ -889,7 +894,6 @@ func newSQLServer(ctx context.Context, cfg sqlServerArgs) (*SQLServer, error) { cfg.Settings, ) ieFactoryMonitor.Start(ctx, pgServer.SQLServer.GetBytesMonitor(), mon.BoundAccount{}) - cfg.stopper.AddCloser(stop.CloserFn(func() { ieFactoryMonitor.Stop(ctx) })) // Now that we have a pgwire.Server (which has a sql.Server), we can close a // circular dependency between the rowexec.Server and sql.Server and set // SessionBoundInternalExecutorFactory. The same applies for setting a