Skip to content

Commit

Permalink
sql: fix flakiness with InternalExecutorFactoryMemMonitor closing Tes…
Browse files Browse the repository at this point in the history
…tServers

We do not actually have to worry about closing the
InternalExecutorFactoryMemMonitor.

Release note: None
  • Loading branch information
RichardJCai committed Jul 12, 2022
1 parent 339149e commit 3c7d6d9
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 5 deletions.
3 changes: 0 additions & 3 deletions pkg/ccl/sqlproxyccl/backend_dialer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
8 changes: 6 additions & 2 deletions pkg/server/server_sql.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
Expand Down

0 comments on commit 3c7d6d9

Please sign in to comment.