Merge #149281

149281: structlogging: clean up hot ranges logger startup procedure r=angles-n-daemons a=angles-n-daemons

structlogging: clean up hot ranges logger startup procedure

A recent fix in #149111 updated the hot ranges logger startup procedure to have the entrypoint for app tenants to be only from the job system. This change neglected to clean up the startup call in the app tenants startup sequence, so this PR removes a lot of that logic (which could be seen as confusing).

Additionally, it changes the job startup sequence so that the job does
not run in single tenant instances, which would be duplicative.

Fixes: none
Epic: none
Release note: none

Co-authored-by: Brian Dillmann <[email protected]>

Author: craig[bot]

pkg/server/server.go

@@ -2361,12 +2361,11 @@ func (s *topLevelServer) AcceptClients(ctx context.Context) error {
 		return err
 	}
 
-	if err := structlogging.StartHotRangesLoggingScheduler(
+	if err := structlogging.StartSystemHotRangesLogger(
 		ctx,
 		s.stopper,
 		s.status,
 		s.ClusterSettings(),
-		nil,
 	); err != nil {
 		return err
 	}

pkg/server/structlogging/BUILD.bazel

@@ -11,7 +11,6 @@ go_library(
     deps = [
         "//pkg/jobs",
         "//pkg/jobs/jobspb",
-        "//pkg/multitenant/tenantcapabilities",
         "//pkg/server/serverpb",
         "//pkg/settings",
         "//pkg/settings/cluster",

pkg/server/structlogging/hot_ranges_log.go

@@ -9,7 +9,6 @@ import (
 	"context"
 	"time"
 
-	"github.com/cockroachdb/cockroach/pkg/multitenant/tenantcapabilities"
 	"github.com/cockroachdb/cockroach/pkg/server/serverpb"
 	"github.com/cockroachdb/cockroach/pkg/settings"
 	"github.com/cockroachdb/cockroach/pkg/settings/cluster"
@@ -80,27 +79,16 @@ type hotRangesLogger struct {
 	lastLogged  time.Time
 }
 
-// StartHotRangesLoggingScheduler starts the hot range log task
+// StartSystemHotRangesLogger starts the hot range log task
 // or job.
 //
 // For system tenants, or single tenant deployments, it runs as
 // a task on each node, logging only the ranges on the node in
-// which it runs. For app tenants in a multi-tenant deployment,
-// it does nothing, allowing the hot range logging job to be the
-// entrypoint.
-func StartHotRangesLoggingScheduler(
-	ctx context.Context,
-	stopper *stop.Stopper,
-	sServer HotRangeGetter,
-	st *cluster.Settings,
-	ti *tenantcapabilities.Entry,
+// which it runs. This function should not be run for app tenants,
+// those will be started via the hot ranges logging job.
+func StartSystemHotRangesLogger(
+	ctx context.Context, stopper *stop.Stopper, sServer HotRangeGetter, st *cluster.Settings,
 ) error {
-	multiTenant := ti != nil && ti.TenantID.IsSet() && !ti.TenantID.IsSystem()
-
-	if multiTenant {
-		return nil
-	}
-
 	logger := hotRangesLogger{
 		sServer:     sServer,
 		st:          st,

pkg/server/structlogging/hot_ranges_log_test.go

@@ -257,7 +257,7 @@ func setupTestServer(
 	structlogging.TelemetryHotRangesStatsInterval.Override(ctx, &settings.SV, time.Millisecond)
 	structlogging.TelemetryHotRangesStatsLoggingDelay.Override(ctx, &settings.SV, 0*time.Millisecond)
 
-	err := structlogging.StartHotRangesLoggingScheduler(ctx, stopper, testHotRangeGetter{}, settings, nil)
+	err := structlogging.StartSystemHotRangesLogger(ctx, stopper, testHotRangeGetter{}, settings)
 	require.NoError(t, err)
 
 	return settings, spy, teardown

pkg/server/tenant.go

@@ -49,7 +49,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/server/serverctl"
 	"github.com/cockroachdb/cockroach/pkg/server/serverpb"
 	"github.com/cockroachdb/cockroach/pkg/server/status"
-	"github.com/cockroachdb/cockroach/pkg/server/structlogging"
 	"github.com/cockroachdb/cockroach/pkg/server/systemconfigwatcher"
 	"github.com/cockroachdb/cockroach/pkg/settings/cluster"
 	"github.com/cockroachdb/cockroach/pkg/spanconfig"
@@ -999,17 +998,6 @@ func (s *SQLServerWrapper) AcceptClients(ctx context.Context) error {
 		}
 	}
 
-	ti, _ := s.sqlServer.tenantConnect.TenantInfo()
-	if err := structlogging.StartHotRangesLoggingScheduler(
-		ctx,
-		s.stopper,
-		s.sqlServer.tenantConnect,
-		s.ClusterSettings(),
-		&ti,
-	); err != nil {
-		return err
-	}
-
 	s.sqlServer.isReady.Store(true)
 
 	log.Event(ctx, "server ready")