From 3306e6ce2fb9b974c5813bb092d1fd998b92062f Mon Sep 17 00:00:00 2001 From: Ridwan Sharif Date: Sat, 20 Oct 2018 15:48:14 -0400 Subject: [PATCH] partitionccl: de-flake TestValidIndexPartitionSetShowZones Resolves #28136 Instead of hard coding table and database IDs, fetch from the `system.namespace` for tables and databases because ID leaks during transaction retries causes test flakiness. Release note: None --- pkg/ccl/backupccl/backup_test.go | 26 +++++--------------------- pkg/ccl/changefeedccl/helpers_test.go | 5 +---- pkg/ccl/partitionccl/zone_test.go | 23 ++++++++++++----------- pkg/sql/ambiguous_commit_test.go | 5 +---- pkg/sql/zone_test.go | 14 ++++++++------ pkg/testutils/sqlutils/table_id.go | 13 +++++++------ 6 files changed, 34 insertions(+), 52 deletions(-) diff --git a/pkg/ccl/backupccl/backup_test.go b/pkg/ccl/backupccl/backup_test.go index be9210b416f3..7e4c15862378 100644 --- a/pkg/ccl/backupccl/backup_test.go +++ b/pkg/ccl/backupccl/backup_test.go @@ -444,21 +444,11 @@ func TestBackupRestoreSystemJobs(t *testing.T) { sanitizedFullDir := localFoo + "/full" fullDir := sanitizedFullDir + "?moarSecretsHere" - backupDatabaseID, err := sqlutils.QueryDatabaseID(sqlDB.DB, "data") - if err != nil { - t.Fatal(err) - } - - backupTableID, err := sqlutils.QueryTableID(sqlDB.DB, "data", "bank") - if err != nil { - t.Fatal(err) - } + backupDatabaseID := sqlutils.QueryDatabaseID(t, sqlDB.DB, "data") + backupTableID := sqlutils.QueryTableID(t, sqlDB.DB, "data", "bank") sqlDB.Exec(t, `CREATE DATABASE restoredb`) - restoreDatabaseID, err := sqlutils.QueryDatabaseID(sqlDB.DB, "restoredb") - if err != nil { - t.Fatal(err) - } + restoreDatabaseID := sqlutils.QueryDatabaseID(t, sqlDB.DB, "restoredb") // We create a full backup so that, below, we can test that incremental // backups sanitize credentials in "INCREMENTAL FROM" URLs. @@ -553,10 +543,7 @@ func checkInProgressBackupRestore( sqlDB.Exec(t, `CREATE DATABASE restoredb`) - backupTableID, err := sqlutils.QueryTableID(sqlDB.DB, "data", "bank") - if err != nil { - t.Fatal(err) - } + backupTableID := sqlutils.QueryTableID(t, sqlDB.DB, "data", "bank") do := func(query string, check inProgressChecker) { jobDone := make(chan error) @@ -791,10 +778,7 @@ func TestBackupRestoreResume(t *testing.T) { restoreDir := "nodelocal:///restore" sqlDB.Exec(t, `BACKUP DATABASE DATA TO $1`, restoreDir) sqlDB.Exec(t, `CREATE DATABASE restoredb`) - restoreDatabaseID, err := sqlutils.QueryDatabaseID(sqlDB.DB, "restoredb") - if err != nil { - t.Fatal(err) - } + restoreDatabaseID := sqlutils.QueryDatabaseID(t, sqlDB.DB, "restoredb") restoreTableID, err := sql.GenerateUniqueDescID(ctx, tc.Servers[0].DB()) if err != nil { t.Fatal(err) diff --git a/pkg/ccl/changefeedccl/helpers_test.go b/pkg/ccl/changefeedccl/helpers_test.go index 8dd7b9a296c4..bd259a4cae73 100644 --- a/pkg/ccl/changefeedccl/helpers_test.go +++ b/pkg/ccl/changefeedccl/helpers_test.go @@ -707,10 +707,7 @@ func forceTableGC( database, table string, ) { t.Helper() - tblID, err := sqlutils.QueryTableID(sqlDB.DB, database, table) - if err != nil { - t.Fatal(err) - } + tblID := sqlutils.QueryTableID(t, sqlDB.DB, database, table) tblKey := roachpb.Key(keys.MakeTablePrefix(tblID)) gcr := roachpb.GCRequest{ diff --git a/pkg/ccl/partitionccl/zone_test.go b/pkg/ccl/partitionccl/zone_test.go index 06ee07c82d3f..8a1ccc4a5cf4 100644 --- a/pkg/ccl/partitionccl/zone_test.go +++ b/pkg/ccl/partitionccl/zone_test.go @@ -50,7 +50,8 @@ func TestValidIndexPartitionSetShowZones(t *testing.T) { partialZoneOverride := *config.NewZoneConfig() partialZoneOverride.GC = &config.GCPolicy{TTLSeconds: 42} - dbDescID := uint32(keys.MinNonPredefinedUserDescID) + dbID := sqlutils.QueryDatabaseID(t, sqlDB.DB, "d") + tableID := sqlutils.QueryTableID(t, sqlDB.DB, "d", "t") defaultRow := sqlutils.ZoneRow{ ID: keys.RootNamespaceID, @@ -63,54 +64,54 @@ func TestValidIndexPartitionSetShowZones(t *testing.T) { Config: zoneOverride, } dbRow := sqlutils.ZoneRow{ - ID: dbDescID, + ID: dbID, CLISpecifier: "d", Config: zoneOverride, } tableRow := sqlutils.ZoneRow{ - ID: dbDescID + 1, + ID: tableID, CLISpecifier: "d.t", Config: zoneOverride, } primaryRow := sqlutils.ZoneRow{ - ID: dbDescID + 1, + ID: tableID, CLISpecifier: "d.t@primary", Config: zoneOverride, } p0Row := sqlutils.ZoneRow{ - ID: dbDescID + 1, + ID: tableID, CLISpecifier: "d.t.p0", Config: zoneOverride, } p1Row := sqlutils.ZoneRow{ - ID: dbDescID + 1, + ID: tableID, CLISpecifier: "d.t.p1", Config: zoneOverride, } // Partially filled config rows partialDbRow := sqlutils.ZoneRow{ - ID: dbDescID, + ID: dbID, CLISpecifier: "d", Config: partialZoneOverride, } partialTableRow := sqlutils.ZoneRow{ - ID: dbDescID + 1, + ID: tableID, CLISpecifier: "d.t", Config: partialZoneOverride, } partialPrimaryRow := sqlutils.ZoneRow{ - ID: dbDescID + 1, + ID: tableID, CLISpecifier: "d.t@primary", Config: partialZoneOverride, } partialP0Row := sqlutils.ZoneRow{ - ID: dbDescID + 1, + ID: tableID, CLISpecifier: "d.t.p0", Config: partialZoneOverride, } partialP1Row := sqlutils.ZoneRow{ - ID: dbDescID + 1, + ID: tableID, CLISpecifier: "d.t.p1", Config: partialZoneOverride, } diff --git a/pkg/sql/ambiguous_commit_test.go b/pkg/sql/ambiguous_commit_test.go index fa7988956fb1..9ed348cf06e4 100644 --- a/pkg/sql/ambiguous_commit_test.go +++ b/pkg/sql/ambiguous_commit_test.go @@ -159,10 +159,7 @@ func TestAmbiguousCommit(t *testing.T) { t.Fatal(err) } - tableID, err := sqlutils.QueryTableID(sqlDB, "test", "t") - if err != nil { - t.Fatal(err) - } + tableID := sqlutils.QueryTableID(t, sqlDB, "test", "t") tableStartKey.Store(keys.MakeTablePrefix(tableID)) // Wait for new table to split & replication. diff --git a/pkg/sql/zone_test.go b/pkg/sql/zone_test.go index c40897886a6e..15db927a3b5a 100644 --- a/pkg/sql/zone_test.go +++ b/pkg/sql/zone_test.go @@ -71,14 +71,17 @@ func TestValidSetShowZones(t *testing.T) { CLISpecifier: "system.jobs", Config: zoneOverride, } - dbDescID := uint32(keys.MinNonPredefinedUserDescID) + + dbID := sqlutils.QueryDatabaseID(t, sqlDB.DB, "d") + tableID := sqlutils.QueryTableID(t, sqlDB.DB, "d", "t") + dbRow := sqlutils.ZoneRow{ - ID: dbDescID, + ID: dbID, CLISpecifier: "d", Config: zoneOverride, } tableRow := sqlutils.ZoneRow{ - ID: dbDescID + 1, + ID: tableID, CLISpecifier: "d.t", Config: zoneOverride, } @@ -99,14 +102,13 @@ func TestValidSetShowZones(t *testing.T) { CLISpecifier: "system.jobs", Config: partialZoneOverride, } - dbDescID = uint32(keys.MinNonPredefinedUserDescID) partialDbRow := sqlutils.ZoneRow{ - ID: dbDescID, + ID: dbID, CLISpecifier: "d", Config: partialZoneOverride, } partialTableRow := sqlutils.ZoneRow{ - ID: dbDescID + 1, + ID: tableID, CLISpecifier: "d.t", Config: partialZoneOverride, } diff --git a/pkg/testutils/sqlutils/table_id.go b/pkg/testutils/sqlutils/table_id.go index 35614c5e29d3..cf5cf2dea9f6 100644 --- a/pkg/testutils/sqlutils/table_id.go +++ b/pkg/testutils/sqlutils/table_id.go @@ -16,23 +16,24 @@ package sqlutils import ( gosql "database/sql" + "testing" ) // QueryDatabaseID returns the database ID of the specified database using the // system.namespace table. -func QueryDatabaseID(sqlDB *gosql.DB, dbName string) (uint32, error) { +func QueryDatabaseID(t testing.TB, sqlDB *gosql.DB, dbName string) uint32 { dbIDQuery := `SELECT id FROM system.namespace WHERE name = $1 AND "parentID" = 0` var dbID uint32 result := sqlDB.QueryRow(dbIDQuery, dbName) if err := result.Scan(&dbID); err != nil { - return 0, err + t.Fatal(err) } - return dbID, nil + return dbID } // QueryTableID returns the table ID of the specified database.table // using the system.namespace table. -func QueryTableID(sqlDB *gosql.DB, dbName, tableName string) (uint32, error) { +func QueryTableID(t testing.TB, sqlDB *gosql.DB, dbName, tableName string) uint32 { tableIDQuery := ` SELECT tables.id FROM system.namespace tables JOIN system.namespace dbs ON dbs.id = tables."parentID" @@ -41,7 +42,7 @@ func QueryTableID(sqlDB *gosql.DB, dbName, tableName string) (uint32, error) { var tableID uint32 result := sqlDB.QueryRow(tableIDQuery, dbName, tableName) if err := result.Scan(&tableID); err != nil { - return 0, err + t.Fatal(err) } - return tableID, nil + return tableID }