Merge cockroachdb#124570

124570: stats: fix TestCreateStatsProgress r=yuzefovich a=yuzefovich This commit fixes a rare flake in `TestCreateStatsProgress` which I think was caused due to the test using "get last job ID" query without specifying the job type. In other words, my hypothesis is that the test could incorrectly pick a job other than CREATE STATS (that happened to be started right after the stats one), and we would never get any progress reported on that other job. This is now fixed by only looking at CREATE STATS jobs. Additionally, the helper function that was only used in this test is now unexported. Fixes: cockroachdb#121770. Release note: None Co-authored-by: Yahor Yuzefovich <[email protected]>
nameisbhaskar · May 23, 2024 · faa4e64 · faa4e64
2 parents f48c4d5 + e7e9dc8
commit faa4e64
Show file tree

Hide file tree

Showing 2 changed files with 9 additions and 11 deletions.
diff --git a/pkg/sql/stats/create_stats_job_test.go b/pkg/sql/stats/create_stats_job_test.go
@@ -389,6 +389,13 @@ func TestCreateStatsProgress(t *testing.T) {
 	}(rowexec.SamplerProgressInterval)
 	rowexec.SamplerProgressInterval = 10
 
+	getLastCreateStatsJobID := func(t testing.TB, db *sqlutils.SQLRunner) jobspb.JobID {
+		var jobID jobspb.JobID
+		db.QueryRow(t, "SELECT id FROM system.jobs WHERE status = 'running' AND "+
+			"job_type = 'CREATE STATS' ORDER BY created DESC LIMIT 1").Scan(&jobID)
+		return jobID
+	}
+
 	var allowRequest chan struct{}
 	var serverArgs base.TestServerArgs
 	filter, setTableID := createStatsRequestFilter(&allowRequest)
@@ -454,7 +461,7 @@ func TestCreateStatsProgress(t *testing.T) {
 	}
 
 	// Fetch the new job ID since we know it's running now.
-	jobID := jobutils.GetLastJobID(t, sqlDB)
+	jobID := getLastCreateStatsJobID(t, sqlDB)
 
 	// Ensure that 0 progress has been recorded since there are no existing
 	// stats available to estimate progress.
@@ -510,7 +517,7 @@ func TestCreateStatsProgress(t *testing.T) {
 	}
 
 	// Fetch the new job ID since we know it's running now.
-	jobID = jobutils.GetLastJobID(t, sqlDB)
+	jobID = getLastCreateStatsJobID(t, sqlDB)
 
 	// Ensure that partial progress has been recorded since there are existing
 	// stats available.

diff --git a/pkg/testutils/jobutils/jobs_verification.go b/pkg/testutils/jobutils/jobs_verification.go
@@ -216,15 +216,6 @@ func GetJobID(t testing.TB, db *sqlutils.SQLRunner, offset int) jobspb.JobID {
 	return jobID
 }
 
-// GetLastJobID gets the most recent job's ID.
-func GetLastJobID(t testing.TB, db *sqlutils.SQLRunner) jobspb.JobID {
-	var jobID jobspb.JobID
-	db.QueryRow(t,
-		"SELECT id FROM system.jobs ORDER BY created DESC LIMIT 1",
-	).Scan(&jobID)
-	return jobID
-}
-
 // GetJobProgress loads the Progress message associated with the job.
 func GetJobProgress(t testing.TB, db *sqlutils.SQLRunner, jobID jobspb.JobID) *jobspb.Progress {
 	ret := &jobspb.Progress{}