Skip to content

Commit

Permalink
Merge cockroachdb#95023
Browse files Browse the repository at this point in the history 
95023: backupccl: cleanup tests that relied on restoring unsupported backup fixtures r=msbutler a=adityamaru

As outlined in https://www.cockroachlabs.com/docs/v22.2/restoring-backups-across-versions.html,
we do not support restoring backups older than the minimum supported binary version of the cluster. For 23.1 this 
minimum supported binary version is going to be 22.2. This patch deletes or modifies existing tests that relied on backup
fixtures outside our minimum restoreable version. Some of the tests that exercised restores of
exceptional backups that could only be generated on pre-22.2 clusters have been deleted since users will have to
restore those backups into 22.2, and take another backup on 22.2 before restoring into 23.1. 

A follow up will enforce the minimum restoreable version during restore job execution.

Informs: cockroachdb#94295

Co-authored-by: adityamaru <[email protected]>
  • Loading branch information
@adityamaru
craig[bot] and adityamaru committed Mar 11, 2023
2 parents 884ff98 + 2883f93 commit 3a7818b
Show file tree
Hide file tree
Showing 1,004 changed files with 428 additions and 1,501 deletions.
107 changes: 107 additions & 0 deletions pkg/ccl/backupccl/backup_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10876,3 +10876,110 @@ func TestExportResponseDataSizeZeroCPUPagination(t *testing.T) {
sqlDB.Exec(t, `BACKUP TABLE foo INTO 'nodelocal://1/foo'`)
require.Equal(t, 2, numRequests)
}

func TestBackupRestoreForeignKeys(t *testing.T) {
defer leaktest.AfterTest(t)()
defer log.Scope(t).Close(t)

params := base.TestServerArgs{}
const numAccounts = 1000
_, sqlDB, _, cleanup := backupRestoreTestSetupWithParams(t, singleNode, numAccounts,
InitManualReplication, base.TestClusterArgs{ServerArgs: params})
defer cleanup()

sqlDB.Exec(t, `CREATE DATABASE test`)
sqlDB.Exec(t, `SET database = test`)
sqlDB.Exec(t, `
CREATE TABLE circular (k INT8 PRIMARY KEY, selfid INT8 UNIQUE);
ALTER TABLE circular ADD CONSTRAINT self_fk FOREIGN KEY (selfid) REFERENCES circular (selfid);
CREATE TABLE parent (k INT8 PRIMARY KEY, j INT8 UNIQUE);
CREATE TABLE child (k INT8 PRIMARY KEY, parent_i INT8 REFERENCES parent, parent_j INT8 REFERENCES parent (j));
CREATE TABLE child_pk (k INT8 PRIMARY KEY REFERENCES parent);
`)

sqlDB.Exec(t, `BACKUP INTO $1 WITH revision_history`, localFoo)

sqlDB.Exec(t, `CREATE TABLE rev_times (id INT PRIMARY KEY, logical_time DECIMAL);`)
sqlDB.Exec(t, `INSERT INTO rev_times VALUES (1, cluster_logical_timestamp());`)

sqlDB.Exec(t, `
CREATE USER newuser;
GRANT ALL ON circular TO newuser;
GRANT ALL ON parent TO newuser;
`)

sqlDB.Exec(t, `BACKUP INTO LATEST IN $1 WITH revision_history`, localFoo)
sqlDB.Exec(t, `INSERT INTO rev_times VALUES (2, cluster_logical_timestamp())`)

sqlDB.Exec(t, `GRANT ALL ON child TO newuser;`)

sqlDB.Exec(t, `BACKUP INTO LATEST IN $1 WITH revision_history`, localFoo)
sqlDB.Exec(t, `INSERT INTO rev_times VALUES (3, cluster_logical_timestamp())`)

sqlDB.Exec(t, `GRANT ALL ON child_pk TO newuser`)

sqlDB.Exec(t, `BACKUP INTO LATEST IN $1 WITH revision_history`, localFoo)

// Test that `SHOW BACKUP` displays the foreign keys correctly.
type testCase struct {
table string
expectedForeignKeyPattern string
}
for _, tc := range []testCase{
{
"circular",
"CONSTRAINT self_fk FOREIGN KEY \\(selfid\\) REFERENCES public\\.circular\\(selfid\\) NOT VALID",
},
{
"child",
"CONSTRAINT \\w+ FOREIGN KEY \\(\\w+\\) REFERENCES public\\.parent\\(\\w+\\)",
},
{
"child_pk",
"CONSTRAINT \\w+ FOREIGN KEY \\(\\w+\\) REFERENCES public\\.parent\\(\\w+\\)",
},
} {
results := sqlDB.QueryStr(t, `
SELECT
create_statement
FROM
[SHOW BACKUP SCHEMAS FROM LATEST IN $1]
WHERE
object_type = 'table' AND object_name = $2
`, localFoo, tc.table)
require.NotEmpty(t, results)
require.Regexp(t, regexp.MustCompile(tc.expectedForeignKeyPattern), results[0][0])
}
sqlDB.Exec(t, `DROP DATABASE test`)

// Test restoring different objects from the backup.
sqlDB.Exec(t, `CREATE DATABASE ts`)
sqlDB.Exec(t, `RESTORE test.rev_times FROM LATEST IN $1 WITH into_db = 'ts'`, localFoo)
for _, ts := range sqlDB.QueryStr(t, `SELECT logical_time FROM ts.rev_times`) {
sqlDB.Exec(t, fmt.Sprintf(`RESTORE DATABASE test FROM LATEST IN $1 AS OF SYSTEM TIME %s`, ts[0]), localFoo)
// Just rendering the constraints loads and validates schema.
sqlDB.Exec(t, `SELECT * FROM pg_catalog.pg_constraint`)
sqlDB.Exec(t, `DROP DATABASE test`)

// Restore a couple tables, including parent but not child_pk.
sqlDB.Exec(t, `CREATE DATABASE test`)
sqlDB.Exec(t, fmt.Sprintf(`RESTORE test.circular FROM LATEST IN $1 AS OF SYSTEM TIME %s`, ts[0]), localFoo)
require.Equal(t, [][]string{
{"test.public.circular", "CREATE TABLE public.circular (\n\tk INT8 NOT NULL,\n\tselfid INT8 NULL,\n\tCONSTRAINT circular_pkey PRIMARY KEY (k ASC),\n\tCONSTRAINT self_fk FOREIGN KEY (selfid) REFERENCES public.circular(selfid) NOT VALID,\n\tUNIQUE INDEX circular_selfid_key (selfid ASC)\n)"},
}, sqlDB.QueryStr(t, `SHOW CREATE TABLE test.circular`))
sqlDB.Exec(t, fmt.Sprintf(`RESTORE test.parent, test.child FROM LATEST IN $1 AS OF SYSTEM TIME %s `, ts[0]), localFoo)
sqlDB.Exec(t, `SELECT * FROM pg_catalog.pg_constraint`)
sqlDB.Exec(t, `DROP DATABASE test`)

// Now do each table on its own with skip_missing_foreign_keys.
sqlDB.Exec(t, `CREATE DATABASE test`)
for _, name := range []string{"child_pk", "child", "circular", "parent"} {
if name == "child" || name == "child_pk" {
sqlDB.ExpectErr(t, "cannot restore table.*without referenced table", fmt.Sprintf(`RESTORE test.%s FROM LATEST IN $1 AS OF SYSTEM TIME %s`, name, ts[0]), localFoo)
}
sqlDB.Exec(t, fmt.Sprintf(`RESTORE test.%s FROM LATEST IN $1 AS OF SYSTEM TIME %s WITH skip_missing_foreign_keys`, name, ts[0]), localFoo)
}
sqlDB.Exec(t, `SELECT * FROM pg_catalog.pg_constraint`)
sqlDB.Exec(t, `DROP DATABASE test`)
}
}
34 changes: 10 additions & 24 deletions pkg/ccl/backupccl/restore_mid_schema_change_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,6 @@ import (
"github.com/stretchr/testify/require"
)

var (
clusterRestoreVersion = version.MustParse("v20.1.0")
noNonMVCCAddSSTable = version.MustParse("v22.1.0")
)

// TestRestoreMidSchemaChanges attempts to RESTORE several BACKUPs that are
// already constructed and store in
// ccl/backupccl/testdata/restore_mid_schema_change. These backups were taken on
Expand Down Expand Up @@ -68,12 +63,12 @@ func TestRestoreMidSchemaChange(t *testing.T) {
testdataBase = datapathutils.TestDataPath(t, "restore_mid_schema_change")
exportDirs = testdataBase + "/exports"
)
for _, isSchemaOnly := range []bool{true, false} {
testutils.RunTrueAndFalse(t, "schema-only", func(t *testing.T, isSchemaOnly bool) {
name := "regular-"
if isSchemaOnly {
name = "schema-only-"
}
for _, isClusterRestore := range []bool{true, false} {
testutils.RunTrueAndFalse(t, "cluster-restore", func(t *testing.T, isClusterRestore bool) {
name = name + "table"
if isClusterRestore {
name = name + "cluster"
Expand All @@ -89,10 +84,6 @@ func TestRestoreMidSchemaChange(t *testing.T) {
clusterVersion, err := parseMajorVersion(clusterVersionDir.Name())
require.NoError(t, err)

if !clusterVersion.AtLeast(clusterRestoreVersion) && isClusterRestore {
continue
}

t.Run(clusterVersionDir.Name(), func(t *testing.T) {
require.True(t, clusterVersionDir.IsDir())
fullClusterVersionDir, err := filepath.Abs(
Expand All @@ -115,8 +106,8 @@ func TestRestoreMidSchemaChange(t *testing.T) {
})
}
})
}
}
})
})
}

// parseMajorVersion parses our major-versioned directory names as if they were
Expand All @@ -139,15 +130,10 @@ func expectedSCJobCount(scName string, ver *version.Version) int {
case "midmultitable":
expNumSCJobs = 2 // this test perform a schema change for each table
case "midprimarykeyswap":
if ver.AtLeast(noNonMVCCAddSSTable) {
// PK change and PK cleanup
expNumSCJobs = 2
} else {
// This will fail so we expect no cleanup job.
expNumSCJobs = 1
}
// PK change and PK cleanup
expNumSCJobs = 2
case "midprimarykeyswapcleanup":
expNumSCJobs = 1
expNumSCJobs = 2
default:
// Most test cases only have 1 schema change under test.
expNumSCJobs = 1
Expand Down Expand Up @@ -257,14 +243,14 @@ func restoreMidSchemaChange(
require.NoError(t, err)

sqlDB.Exec(t, "USE defaultdb")
restoreQuery := "RESTORE defaultdb.* FROM $1"
restoreQuery := "RESTORE defaultdb.* FROM LATEST IN $1"
if isClusterRestore {
restoreQuery = "RESTORE FROM $1"
restoreQuery = "RESTORE FROM LATEST IN $1"
}
if isSchemaOnly {
restoreQuery = restoreQuery + "with schema_only"
}
log.Infof(context.Background(), "%+v", sqlDB.QueryStr(t, "SHOW BACKUP $1", localFoo))
log.Infof(context.Background(), "%+v", sqlDB.QueryStr(t, "SHOW BACKUP LATEST IN $1", localFoo))
sqlDB.Exec(t, restoreQuery, localFoo)
// Wait for all jobs to terminate. Some may fail since we don't restore
// adding spans.
Expand Down
8 changes: 6 additions & 2 deletions pkg/ccl/backupccl/restore_old_sequences_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ import (
"testing"

"github.com/cockroachdb/cockroach/pkg/base"
"github.com/cockroachdb/cockroach/pkg/clusterversion"
"github.com/cockroachdb/cockroach/pkg/settings/cluster"
"github.com/cockroachdb/cockroach/pkg/testutils/datapathutils"
"github.com/cockroachdb/cockroach/pkg/util/leaktest"
"github.com/cockroachdb/cockroach/pkg/util/log"
Expand All @@ -30,7 +32,7 @@ import (
//
// VERSION=...
// roachprod create local
// roachprod wipe localÅ
// roachprod wipe local
// roachprod stage local release ${VERSION}
// roachprod start local
// roachprod sql local:1 -- -e "$(cat pkg/ccl/backupccl/testdata/restore_old_sequences/create.sql)"
Expand Down Expand Up @@ -66,6 +68,8 @@ func TestRestoreOldSequences(t *testing.T) {
func restoreOldSequencesTest(exportDir string, isSchemaOnly bool) func(t *testing.T) {
return func(t *testing.T) {
params := base.TestServerArgs{}
params.Settings = cluster.MakeTestingClusterSettingsWithVersions(clusterversion.TestingBinaryVersion,
clusterversion.TestingBinaryMinSupportedVersion, false /* initializeVersion */)
const numAccounts = 1000
_, sqlDB, dir, cleanup := backupRestoreTestSetupWithParams(t, singleNode, numAccounts,
InitManualReplication, base.TestClusterArgs{ServerArgs: params})
Expand All @@ -75,7 +79,7 @@ func restoreOldSequencesTest(exportDir string, isSchemaOnly bool) func(t *testin
sqlDB.Exec(t, `CREATE DATABASE test`)
var unused string
var importedRows int
restoreQuery := `RESTORE test.* FROM $1`
restoreQuery := `RESTORE test.* FROM LATEST IN $1`
if isSchemaOnly {
restoreQuery = restoreQuery + " with schema_only"
}
Expand Down
Loading

0 comments on commit 3a7818b

Please sign in to comment.