From db5f5db46a87b3de4409d61a683e403e6b271a78 Mon Sep 17 00:00:00 2001 From: Rebecca Taft Date: Tue, 5 Feb 2019 14:08:12 -0500 Subject: [PATCH] importccl: fix flaky test TestImportCSVStmt TestImportCSVStmt tests that IMPORT jobs appear in a certain order in the system.jobs table. Automatic statistics were causing this test to be flaky since CreateStats jobs were present in the jobs table as well, in an unpredictable order. This commit fixes the problem by only selecting IMPORT jobs from the jobs table. Fixes #34568 Release note: None --- pkg/ccl/backupccl/backup_test.go | 7 ++---- pkg/ccl/importccl/import_stmt_test.go | 23 +++++++++-------- pkg/testutils/jobutils/jobs_verification.go | 28 +++++++-------------- 3 files changed, 23 insertions(+), 35 deletions(-) diff --git a/pkg/ccl/backupccl/backup_test.go b/pkg/ccl/backupccl/backup_test.go index ac92f3c49223..a2ecbc816521 100644 --- a/pkg/ccl/backupccl/backup_test.go +++ b/pkg/ccl/backupccl/backup_test.go @@ -435,9 +435,6 @@ func TestBackupRestoreSystemJobs(t *testing.T) { conn := sqlDB.DB.(*gosql.DB) defer cleanupFn() - // Get the number of existing jobs. - baseNumJobs := jobutils.GetSystemJobsCount(t, sqlDB) - sanitizedIncDir := localFoo + "/inc" incDir := sanitizedIncDir + "?secretCredentialsHere" @@ -462,7 +459,7 @@ func TestBackupRestoreSystemJobs(t *testing.T) { sqlDB.Exec(t, `SET DATABASE = data`) sqlDB.Exec(t, `BACKUP TABLE bank TO $1 INCREMENTAL FROM $2`, incDir, fullDir) - if err := jobutils.VerifySystemJob(t, sqlDB, baseNumJobs+1, jobspb.TypeBackup, jobs.StatusSucceeded, jobs.Record{ + if err := jobutils.VerifySystemJob(t, sqlDB, 1, jobspb.TypeBackup, jobs.StatusSucceeded, jobs.Record{ Username: security.RootUser, Description: fmt.Sprintf( `BACKUP TABLE bank TO '%s' INCREMENTAL FROM '%s'`, @@ -477,7 +474,7 @@ func TestBackupRestoreSystemJobs(t *testing.T) { } sqlDB.Exec(t, `RESTORE TABLE bank FROM $1, $2 WITH OPTIONS ('into_db'='restoredb')`, fullDir, incDir) - if err := jobutils.VerifySystemJob(t, sqlDB, baseNumJobs+2, jobspb.TypeRestore, jobs.StatusSucceeded, jobs.Record{ + if err := jobutils.VerifySystemJob(t, sqlDB, 0, jobspb.TypeRestore, jobs.StatusSucceeded, jobs.Record{ Username: security.RootUser, Description: fmt.Sprintf( `RESTORE TABLE bank FROM '%s', '%s' WITH into_db = 'restoredb'`, diff --git a/pkg/ccl/importccl/import_stmt_test.go b/pkg/ccl/importccl/import_stmt_test.go index 47b1f214115f..20a8311d50b8 100644 --- a/pkg/ccl/importccl/import_stmt_test.go +++ b/pkg/ccl/importccl/import_stmt_test.go @@ -867,13 +867,18 @@ func TestImportCSVStmt(t *testing.T) { if testing.Short() { t.Skip("short") } - t.Skip(`#34568`) - const ( - nodes = 3 - numFiles = nodes + 2 - rowsPerFile = 1000 - ) + const nodes = 3 + + numFiles := nodes + 2 + rowsPerFile := 1000 + if util.RaceEnabled { + // This test takes a while with the race detector, so reduce the number of + // files and rows per file in an attempt to speed it up. + numFiles = nodes + rowsPerFile = 16 + } + ctx := context.Background() dir, cleanup := testutils.TempDir(t) defer cleanup() @@ -897,9 +902,6 @@ func TestImportCSVStmt(t *testing.T) { t.Fatal(err) } - // Get the number of existing jobs. - baseNumJobs := jobutils.GetSystemJobsCount(t, sqlDB) - if err := ioutil.WriteFile(filepath.Join(dir, "empty.csv"), nil, 0666); err != nil { t.Fatal(err) } @@ -1174,7 +1176,7 @@ func TestImportCSVStmt(t *testing.T) { } jobPrefix += `t (a INT8 PRIMARY KEY, b STRING, INDEX (b), INDEX (a, b)) CSV DATA (%s)` - if err := jobutils.VerifySystemJob(t, sqlDB, baseNumJobs+testNum, jobspb.TypeImport, jobs.StatusSucceeded, jobs.Record{ + if err := jobutils.VerifySystemJob(t, sqlDB, testNum, jobspb.TypeImport, jobs.StatusSucceeded, jobs.Record{ Username: security.RootUser, Description: fmt.Sprintf(jobPrefix+tc.jobOpts, strings.Join(tc.files, ", ")), }); err != nil { @@ -1188,7 +1190,6 @@ func TestImportCSVStmt(t *testing.T) { t, "does not exist", `SELECT count(*) FROM t`, ) - testNum++ sqlDB.QueryRow( t, `RESTORE csv.* FROM $1 WITH into_db = $2`, backupPath, intodb, ).Scan( diff --git a/pkg/testutils/jobutils/jobs_verification.go b/pkg/testutils/jobutils/jobs_verification.go index e5292aaf6a06..0f26fd9ff0d2 100644 --- a/pkg/testutils/jobutils/jobs_verification.go +++ b/pkg/testutils/jobutils/jobs_verification.go @@ -118,25 +118,17 @@ func BulkOpResponseFilter(allowProgressIota *chan struct{}) storagebase.ReplicaR } } -// GetSystemJobsCount queries the number of entries in the jobs table. -func GetSystemJobsCount(t testing.TB, db *sqlutils.SQLRunner) int { - var jobCount int - db.QueryRow(t, `SELECT count(*) FROM crdb_internal.jobs`).Scan(&jobCount) - return jobCount -} - func verifySystemJob( t testing.TB, db *sqlutils.SQLRunner, offset int, - expectedType jobspb.Type, + filterType jobspb.Type, expectedStatus string, expectedRunningStatus string, expected jobs.Record, ) error { var actual jobs.Record var rawDescriptorIDs pq.Int64Array - var actualType string var statusString string var runningStatus gosql.NullString var runningStatusString string @@ -144,11 +136,12 @@ func verifySystemJob( // because job-generating SQL queries (e.g. BACKUP) do not currently return // the job ID. db.QueryRow(t, ` - SELECT job_type, description, user_name, descriptor_ids, status, running_status - FROM crdb_internal.jobs ORDER BY created LIMIT 1 OFFSET $1`, + SELECT description, user_name, descriptor_ids, status, running_status + FROM crdb_internal.jobs WHERE job_type = $1 ORDER BY created LIMIT 1 OFFSET $2`, + filterType.String(), offset, ).Scan( - &actualType, &actual.Description, &actual.Username, &rawDescriptorIDs, + &actual.Description, &actual.Username, &rawDescriptorIDs, &statusString, &runningStatus, ) if runningStatus.Valid { @@ -173,9 +166,6 @@ func verifySystemJob( return errors.Errorf("job %d: expected running status %v, got %v", offset, expectedRunningStatus, runningStatusString) } - if e, a := expectedType.String(), actualType; e != a { - return errors.Errorf("job %d: expected type %v, got type %v", offset, e, a) - } return nil } @@ -186,11 +176,11 @@ func VerifyRunningSystemJob( t testing.TB, db *sqlutils.SQLRunner, offset int, - expectedType jobspb.Type, + filterType jobspb.Type, expectedRunningStatus jobs.RunningStatus, expected jobs.Record, ) error { - return verifySystemJob(t, db, offset, expectedType, "running", string(expectedRunningStatus), expected) + return verifySystemJob(t, db, offset, filterType, "running", string(expectedRunningStatus), expected) } // VerifySystemJob checks that job records are created as expected. @@ -198,11 +188,11 @@ func VerifySystemJob( t testing.TB, db *sqlutils.SQLRunner, offset int, - expectedType jobspb.Type, + filterType jobspb.Type, expectedStatus jobs.Status, expected jobs.Record, ) error { - return verifySystemJob(t, db, offset, expectedType, string(expectedStatus), "", expected) + return verifySystemJob(t, db, offset, filterType, string(expectedStatus), "", expected) } // GetJobID gets a particular job's ID.