Merge #149428

149428: structlogging: fix hot ranges logging job exit mechanism r=angles-n-daemons a=angles-n-daemons

We configured the hot ranges logging job (to be released in 25.3) as a forever background job, which means its meant to run forever, and only for app tenants.

However there were two issues with our current implementation. The first is that it runs both for app tenants and system tenants, and second is that for the app tenants, on quiescence it exists with a nil error.

In order to run forever, quiescence requires the job to error, so that it is picked up by another node when a node is exiting.

Fixes: none
Epic: none
Release note: none

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

Author: craig[bot]

pkg/server/structlogging/BUILD.bazel

@@ -29,6 +29,7 @@ go_library(
 go_test(
     name = "structlogging_test",
     srcs = [
+        "hot_ranges_log_job_test.go",
         "hot_ranges_log_test.go",
         "main_test.go",
     ],
@@ -38,6 +39,8 @@ go_test(
     }),
     deps = [
         ":structlogging",
+        "//pkg/base",
+        "//pkg/jobs",
         "//pkg/security/securityassets",
         "//pkg/security/securitytest",
         "//pkg/server",

pkg/server/structlogging/hot_ranges_log_job.go

@@ -37,14 +37,22 @@ func (j *hotRangesLoggingJob) Resume(ctx context.Context, execCtxI interface{})
 
 	jobExec := execCtxI.(sql.JobExecContext)
 	execCfg := jobExec.ExecCfg()
+
+	// Do not run this job for the system tenant.
+	if execCfg.Codec.ForSystemTenant() {
+		return nil
+	}
+
 	logger := &hotRangesLogger{
 		sServer:     execCfg.TenantStatusServer,
 		st:          j.settings,
 		multiTenant: true,
 		lastLogged:  timeutil.Now(),
 	}
 	logger.start(ctx, execCfg.Stopper)
-	return nil
+
+	// Signal to the job system to pick this job back up.
+	return jobs.MarkAsRetryJobError(errors.New("failing hot ranges job so that it is restarted"))
 }
 
 func (j *hotRangesLoggingJob) OnFailOrCancel(

pkg/server/structlogging/hot_ranges_log_job_test.go

@@ -0,0 +1,57 @@
+// Copyright 2025 The Cockroach Authors.
+//
+// Use of this software is governed by the CockroachDB Software License
+// included in the /LICENSE file.
+
+package structlogging_test
+
+import (
+	"context"
+	"testing"
+
+	"github.com/cockroachdb/cockroach/pkg/base"
+	"github.com/cockroachdb/cockroach/pkg/jobs"
+	"github.com/cockroachdb/cockroach/pkg/testutils"
+	"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
+	"github.com/cockroachdb/cockroach/pkg/testutils/skip"
+	"github.com/cockroachdb/cockroach/pkg/util/leaktest"
+	"github.com/cockroachdb/cockroach/pkg/util/log"
+	"github.com/cockroachdb/errors"
+	"github.com/stretchr/testify/require"
+)
+
+// This test verifies that the hot ranges logging job starts for both
+// the system and app layers, and exits quickly for the system layer.
+func TestHotRangesLoggingJobExitProcedure(t *testing.T) {
+	defer leaktest.AfterTest(t)()
+	defer log.Scope(t).Close(t)
+	skip.UnderStress(t)
+	skip.UnderRace(t)
+
+	ctx := context.Background()
+	ts := serverutils.StartServerOnly(t, base.TestServerArgs{
+		Knobs: base.TestingKnobs{
+			JobsTestingKnobs: jobs.NewTestingKnobsWithShortIntervals(),
+		},
+	})
+	defer ts.Stopper().Stop(ctx)
+
+	syslayer := ts.SystemLayer().SQLConn(t)
+	applayer := ts.ApplicationLayer().SQLConn(t)
+
+	testutils.SucceedsSoon(t, func() error {
+		row := syslayer.QueryRow("SELECT status FROM system.public.jobs WHERE id = $1", jobs.HotRangesLoggerJobID)
+		var status string
+		require.NoError(t, row.Scan(&status))
+		if status != "succeeded" {
+			return errors.Newf("system job status is %s, not succeeded", status)
+		}
+
+		row = applayer.QueryRow("SELECT status FROM system.public.jobs WHERE id = $1", jobs.HotRangesLoggerJobID)
+		require.NoError(t, row.Scan(&status))
+		if status != "running" {
+			return errors.Newf("app job status is %s, not running", status)
+		}
+		return nil
+	})
+}