Merge #97991 #98112

97991: server: move settingswatcher initialization to the start r=JeffSwenson a=JeffSwenson

Previously, the settingswatcher was one of the last services initialized during sql server start up. Now, it is one of the first services to start up. The settings do not have a well defined value until after settingwatcher initializes. Some settings, like cluster version, must have valid values during initialization.

Part of #94843

98112: sql/schmachanger: use more friendly language for fallback r=fqazi a=fqazi

Previously, the declarative schema changer used slightly
alarming language when falling back. To address this,
this patch removes the word failure and uses softer
language. Also, the logging level is reduced to remove
this message by default.

Fixes: #97922

Release note: None

Co-authored-by: Jeff <[email protected]>
Co-authored-by: Faizan Qazi <[email protected]>

pkg/server/server_sql.go

@@ -526,6 +526,19 @@ func newSQLServer(ctx context.Context, cfg sqlServerArgs) (*SQLServer, error) {
 	// SQL pod server.
 	_, isMixedSQLAndKVNode := cfg.nodeIDContainer.OptionalNodeID()
 
+	var settingsWatcher *settingswatcher.SettingsWatcher
+	if codec.ForSystemTenant() {
+		settingsWatcher = settingswatcher.New(
+			cfg.clock, codec, cfg.Settings, cfg.rangeFeedFactory, cfg.stopper, cfg.settingsStorage,
+		)
+	} else {
+		// Create the tenant settings watcher, using the tenant connector as the
+		// overrides monitor.
+		settingsWatcher = settingswatcher.NewWithOverrides(
+			cfg.clock, codec, cfg.Settings, cfg.rangeFeedFactory, cfg.stopper, cfg.tenantConnect, cfg.settingsStorage,
+		)
+	}
+
 	sqllivenessKnobs, _ := cfg.TestingKnobs.SQLLivenessKnobs.(*sqlliveness.TestingKnobs)
 	var sessionEventsConsumer slinstance.SessionEventListener
 	if !isMixedSQLAndKVNode {
@@ -1303,19 +1316,6 @@ func newSQLServer(ctx context.Context, cfg sqlServerArgs) (*SQLServer, error) {
 	vmoduleSetting.SetOnChange(&cfg.Settings.SV, fn)
 	fn(ctx)
 
-	var settingsWatcher *settingswatcher.SettingsWatcher
-	if codec.ForSystemTenant() {
-		settingsWatcher = settingswatcher.New(
-			cfg.clock, codec, cfg.Settings, cfg.rangeFeedFactory, cfg.stopper, cfg.settingsStorage,
-		)
-	} else {
-		// Create the tenant settings watcher, using the tenant connector as the
-		// overrides monitor.
-		settingsWatcher = settingswatcher.NewWithOverrides(
-			cfg.clock, codec, cfg.Settings, cfg.rangeFeedFactory, cfg.stopper, cfg.tenantConnect, cfg.settingsStorage,
-		)
-	}
-
 	return &SQLServer{
 		ambientCtx:                     cfg.BaseConfig.AmbientCtx,
 		stopper:                        cfg.stopper,
@@ -1371,6 +1371,13 @@ func (s *SQLServer) preStart(
 		}
 	}
 
+	// Initialize the settings watcher early in sql server startup. Settings
+	// values are meaningless before the watcher is initialized and most sub
+	// systems depend on system settings.
+	if err := s.settingsWatcher.Start(ctx); err != nil {
+		return errors.Wrap(err, "initializing settings")
+	}
+
 	// Load the multi-region enum by reading the system database's descriptor.
 	// This also serves as a simple check to see if a tenant exist (i.e. by
 	// checking whether the system db has been bootstrapped).
@@ -1505,11 +1512,8 @@ func (s *SQLServer) preStart(
 		bootstrapVersion = roachpb.Version{Major: 20, Minor: 1, Internal: 1}
 	}
 
-	if err := s.settingsWatcher.Start(ctx); err != nil {
-		return errors.Wrap(err, "initializing settings")
-	}
 	if err := s.systemConfigWatcher.Start(ctx, s.stopper); err != nil {
-		return errors.Wrap(err, "initializing settings")
+		return errors.Wrap(err, "initializing system config watcher")
 	}
 
 	clusterVersionMetrics := clusterversion.MakeMetricsAndRegisterOnVersionChangeCallback(&s.cfg.Settings.SV)

pkg/sql/schemachanger/scerrors/errors.go

@@ -75,7 +75,7 @@ func (el EventLogger) HandlePanicAndLogError(ctx context.Context, err *error) {
 			log.InfofDepth(ctx, depth, "done %s in %s", el.msg, redact.Safe(timeutil.Since(el.start)))
 		}
 	case HasNotImplemented(*err):
-		log.InfofDepth(ctx, depth, "failed %s with error: %v", el.msg, *err)
+		log.VEventfDepth(ctx, depth, 1, "declarative schema changer does not support %s: %v", el.msg, *err)
 	case errors.HasAssertionFailure(*err):
 		*err = errors.Wrapf(*err, "%s", el.msg)
 		fallthrough