From 8bfcc91081b327909c0326be2ca3777641cd0a14 Mon Sep 17 00:00:00 2001 From: David Taylor Date: Mon, 22 Dec 2025 22:10:46 +0000 Subject: [PATCH] server: init external storage before SQL start SQL start registers in liveness and invites distSQL flows to be scheduled, which may expect initialized, ready external storage. As such, init external storage before SQL start. External storage can end up coming back and wanting to run SQL (for userfile) so this remains somewhat circular, but given a SQL flow or job or query is what would end up actually opening storage, this ordering seems less fraught than starting SQL first. Release note: none. Epic: none. Unclear if this will fix all cases of observed errors here since we don't have a reproduction or failing test, so this is more of just an optimistic cleanup related to, but not yet known to close, #120022. --- pkg/server/server.go | 40 ++++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/pkg/server/server.go b/pkg/server/server.go index 8dcb698b10ed..a39244321ac4 100644 --- a/pkg/server/server.go +++ b/pkg/server/server.go @@ -2137,6 +2137,26 @@ func (s *topLevelServer) PreStart(ctx context.Context) error { // executes a SQL query, this must be done after the SQL layer is ready. s.node.recordJoinEvent(ctx) + // Initialize the external storage builders configuration params now that the + // engines have been created. The object can be used to create ExternalStorage + // objects hereafter. + ieMon := sql.MakeInternalExecutorMemMonitor(sql.MemoryMetrics{}, s.ClusterSettings()) + ieMon.StartNoReserved(ctx, s.PGServer().SQLServer.GetBytesMonitor()) + s.stopper.AddCloser(stop.CloserFn(func() { ieMon.Stop(ctx) })) + s.externalStorageBuilder.init( + s.cfg.EarlyBootExternalStorageAccessor, + s.cfg.ExternalIODirConfig, + s.st, + s.sqlServer.sqlIDContainer, + s.kvNodeDialer, + s.cfg.TestingKnobs, + true, /* allowLocalFastPath */ + s.sqlServer.execCfg.InternalDB.CloneWithMemoryMonitor(sql.MemoryMetrics{}, ieMon), + nil, /* TenantExternalIORecorder */ + s.appRegistry, + s.cfg.ExternalIODir, + ) + if !s.cfg.DisableSQLServer { // Start the SQL subsystem. if err := s.sqlServer.preStart( @@ -2199,26 +2219,6 @@ func (s *topLevelServer) PreStart(ctx context.Context) error { return err } - // Initialize the external storage builders configuration params now that the - // engines have been created. The object can be used to create ExternalStorage - // objects hereafter. - ieMon := sql.MakeInternalExecutorMemMonitor(sql.MemoryMetrics{}, s.ClusterSettings()) - ieMon.StartNoReserved(ctx, s.PGServer().SQLServer.GetBytesMonitor()) - s.stopper.AddCloser(stop.CloserFn(func() { ieMon.Stop(ctx) })) - s.externalStorageBuilder.init( - s.cfg.EarlyBootExternalStorageAccessor, - s.cfg.ExternalIODirConfig, - s.st, - s.sqlServer.sqlIDContainer, - s.kvNodeDialer, - s.cfg.TestingKnobs, - true, /* allowLocalFastPath */ - s.sqlServer.execCfg.InternalDB.CloneWithMemoryMonitor(sql.MemoryMetrics{}, ieMon), - nil, /* TenantExternalIORecorder */ - s.appRegistry, - s.cfg.ExternalIODir, - ) - if err := s.runIdempontentSQLForInitType(ctx, state.initType); err != nil { return err }