Skip to content

Commit 0dbe5b5

Browse files
committed
ccl/schemachanger: avoid table restore flavors in mixed version
Previously, the schema changer CCL backup / restore tests could timeout due to complex depth of coverage offered. To address this, this patch will avoid testing per-table restore flavors for multiregion and mixed version variants, so that fewer permutations are tested. Fixes: #160314 Fixes: #160275 Release note: None
1 parent 31e1f4c commit 0dbe5b5

File tree

1 file changed

+32
-18
lines changed

1 file changed

+32
-18
lines changed

pkg/sql/schemachanger/sctest/backup.go

Lines changed: 32 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ func BackupRollbacks(t *testing.T, path string, factory TestServerFactory) {
6969
// cannot be created with schema_locked by default yet.
7070
factory = factory.WithSchemaLockDisabled()
7171
cumulativeTestForEachPostCommitStage(t, path, factory, nil, func(t *testing.T, cs CumulativeTestCaseSpec) {
72-
backupRollbacks(t, factory, cs)
72+
backupRollbacks(t, factory, cs, false /*isMixedVersion*/)
7373
},
7474
nil /* samplingFn */)
7575
}
@@ -96,6 +96,11 @@ func BackupSuccessMixedVersion(t *testing.T, path string, factory TestServerFact
9696
cumulativeTestForEachPostCommitStage(t, path, factory, func(t *testing.T, spec CumulativeTestSpec, dbName string) {
9797
backupArgs = backupSuccessPrepare(t, factory, spec, dbName)
9898
}, func(t *testing.T, cs CumulativeTestCaseSpec) {
99+
if backupArgs.isMultiRegion {
100+
// Speed up the mixed version multiregion cases by excluding restores of individual
101+
// tables in the mixed version state..
102+
backupArgs.excludeAllTablesInDatabaseFlavor = true
103+
}
99104
backupSuccess(t, backupArgs, cs)
100105
},
101106
nil /* samplingFn */)
@@ -117,7 +122,7 @@ func BackupRollbacksMixedVersion(t *testing.T, path string, factory TestServerFa
117122
// disabling schema_locked in a mixed version state yet.
118123
factory = factory.WithSchemaLockDisabled()
119124
cumulativeTestForEachPostCommitStage(t, path, factory, nil, func(t *testing.T, cs CumulativeTestCaseSpec) {
120-
backupRollbacks(t, factory, cs)
125+
backupRollbacks(t, factory, cs, true /*isMixedVersion*/)
121126
}, nil /* samplingFn */)
122127
}
123128

@@ -171,11 +176,13 @@ type backupInfo struct {
171176

172177
// backupSuccessTestArgs are arguments generated by backupSuccessPrepare
173178
type backupSuccessTestArgs struct {
174-
server TestServer
175-
backups map[backupKey]backupInfo
176-
before [][]string
177-
after [][]string
178-
pe PlanExplainer
179+
server TestServer
180+
backups map[backupKey]backupInfo
181+
before [][]string
182+
after [][]string
183+
pe PlanExplainer
184+
isMultiRegion bool
185+
excludeAllTablesInDatabaseFlavor bool
179186
}
180187

181188
// backupSuccessPrepare setups a server and takes all required backups for
@@ -276,12 +283,11 @@ func backupSuccessPrepare(
276283
})
277284
// Take all the required backups using the timestamps gathered.
278285
numStagesIncluded := 0
279-
isMultiRegion := false
280-
tdb.QueryRow(t, "SELECT count(*) >1 FROM [SHOW REGIONS]").Scan(&isMultiRegion)
286+
tdb.QueryRow(t, "SELECT count(*) >1 FROM [SHOW REGIONS]").Scan(&backupArgs.isMultiRegion)
281287
for key := range backupsToTake {
282288
// Determine if we have too many backups to test. We will sample
283289
// at most 8 stages or 60% stages, whichever is smaller.
284-
if shouldSkipBackup(numStagesIncluded, len(backupsToTake), isMultiRegion) {
290+
if shouldSkipBackup(numStagesIncluded, len(backupsToTake), backupArgs.isMultiRegion) {
285291
backupArgs.backups[key] = backupInfo{}
286292
continue
287293
}
@@ -329,12 +335,13 @@ func backupSuccess(t *testing.T, args backupSuccessTestArgs, cs CumulativeTestCa
329335
skip.IgnoreLintf(t, "no backup for %v", key)
330336
}
331337
b := backupRestoreOutcome{
332-
url: info.url,
333-
maySucceed: true,
334-
expectedOnSuccess: args.after,
335-
skipComparisonOnSuccess: skipOutputComparison,
336-
mayRollback: false,
337-
expectedOnRollback: args.before,
338+
url: info.url,
339+
maySucceed: true,
340+
expectedOnSuccess: args.after,
341+
skipComparisonOnSuccess: skipOutputComparison,
342+
mayRollback: false,
343+
expectedOnRollback: args.before,
344+
excludeAllTablesInDatabaseFlavor: args.excludeAllTablesInDatabaseFlavor,
338345
}
339346
tdb := sqlutils.MakeSQLRunner(args.server.DB)
340347
if info.isPostBackfill {
@@ -362,7 +369,9 @@ func backupSuccess(t *testing.T, args backupSuccessTestArgs, cs CumulativeTestCa
362369
exerciseBackupRestore(t, tdb, cs, b, args.pe)
363370
}
364371

365-
func backupRollbacks(t *testing.T, factory TestServerFactory, cs CumulativeTestCaseSpec) {
372+
func backupRollbacks(
373+
t *testing.T, factory TestServerFactory, cs CumulativeTestCaseSpec, isMixedVersion bool,
374+
) {
366375
if cs.Phase != scop.PostCommitPhase {
367376
return
368377
}
@@ -443,6 +452,8 @@ func backupRollbacks(t *testing.T, factory TestServerFactory, cs CumulativeTestC
443452

444453
// Check that it's the same as before the schema change was attempted.
445454
require.Equal(t, expected, postRollback, "rolled back schema change should be no-op")
455+
isMultiRegion := false
456+
tdb.QueryRow(t, "SELECT count(*) >1 FROM [SHOW REGIONS]").Scan(&isMultiRegion)
446457

447458
// Restore the backups of the database taken mid-rolled-back-schema-change
448459
// in various ways, check that they end up in the same state as present.
@@ -451,6 +462,8 @@ func backupRollbacks(t *testing.T, factory TestServerFactory, cs CumulativeTestC
451462
url: url,
452463
mayRollback: true,
453464
expectedOnRollback: postRollback,
465+
// Speed up mixed version variants by excluding individual table restores.
466+
excludeAllTablesInDatabaseFlavor: isMultiRegion && isMixedVersion,
454467
}
455468
var name string
456469
switch i {
@@ -482,6 +495,7 @@ type backupRestoreOutcome struct {
482495
maySucceed, mayRollback bool
483496
expectedOnSuccess, expectedOnRollback [][]string
484497
skipComparisonOnSuccess bool
498+
excludeAllTablesInDatabaseFlavor bool
485499
}
486500

487501
func exerciseBackupRestore(
@@ -538,7 +552,7 @@ func exerciseBackupRestore(
538552
}
539553

540554
// Determine which tables to restore when restoring tables only.
541-
{
555+
if !b.excludeAllTablesInDatabaseFlavor {
542556
var tablesToRestore []string
543557
Loop:
544558
for _, row := range backupContents {

0 commit comments

Comments
 (0)