From db08353fb1b883595b6ef77bc6551966a356757a Mon Sep 17 00:00:00 2001 From: Michael Butler Date: Wed, 9 Mar 2022 20:08:02 -0500 Subject: [PATCH] backupccl: add random data to tables in TestBackupRestoreDataRoundtrips Previously, TestBackupRestoreDataRoundtrips did not test if data in the randomly created tables were preserved after a backup restore roundtrip. This patch tries to add random data to each table and tests that the data is preserved across the roundtrip. In addition, this pr moves this test to its own package because the test takes a long time to run. Release note: None --- pkg/BUILD.bazel | 1 + pkg/ccl/backupccl/BUILD.bazel | 2 - pkg/ccl/backupccl/backup_test.go | 5 +- pkg/ccl/backupccl/backuprand/BUILD.bazel | 30 +++++++++ .../{ => backuprand}/backup_rand_test.go | 58 ++++++++++-------- pkg/ccl/backupccl/backuprand/main_test.go | 34 +++++++++++ pkg/ccl/backupccl/backuputils/BUILD.bazel | 2 + pkg/ccl/backupccl/backuputils/testutils.go | 61 +++++++++++++++++++ pkg/ccl/backupccl/utils_test.go | 57 ----------------- 9 files changed, 163 insertions(+), 87 deletions(-) create mode 100644 pkg/ccl/backupccl/backuprand/BUILD.bazel rename pkg/ccl/backupccl/{ => backuprand}/backup_rand_test.go (74%) create mode 100644 pkg/ccl/backupccl/backuprand/main_test.go diff --git a/pkg/BUILD.bazel b/pkg/BUILD.bazel index 94df05398953..de18c792970a 100644 --- a/pkg/BUILD.bazel +++ b/pkg/BUILD.bazel @@ -12,6 +12,7 @@ ALL_TESTS = [ "//pkg/ccl/backupccl/backupdest:backupdest_test", "//pkg/ccl/backupccl/backupinfo:backupinfo_disallowed_imports_test", "//pkg/ccl/backupccl/backupinfo:backupinfo_test", + "//pkg/ccl/backupccl/backuprand:backuprand_test", "//pkg/ccl/backupccl/backupresolver:backupresolver_test", "//pkg/ccl/backupccl:backupccl_test", "//pkg/ccl/baseccl:baseccl_test", diff --git a/pkg/ccl/backupccl/BUILD.bazel b/pkg/ccl/backupccl/BUILD.bazel index d99b56820c85..88a810fe1190 100644 --- a/pkg/ccl/backupccl/BUILD.bazel +++ b/pkg/ccl/backupccl/BUILD.bazel @@ -143,7 +143,6 @@ go_test( "backup_intents_test.go", "backup_metadata_test.go", "backup_planning_test.go", - "backup_rand_test.go", "backup_tenant_test.go", "backup_test.go", "bench_covering_test.go", @@ -194,7 +193,6 @@ go_test( "//pkg/clusterversion", "//pkg/config", "//pkg/config/zonepb", - "//pkg/internal/sqlsmith", "//pkg/jobs", "//pkg/jobs/jobspb", "//pkg/jobs/jobsprotectedts", diff --git a/pkg/ccl/backupccl/backup_test.go b/pkg/ccl/backupccl/backup_test.go index b04b2932af31..d5aae009ac47 100644 --- a/pkg/ccl/backupccl/backup_test.go +++ b/pkg/ccl/backupccl/backup_test.go @@ -41,6 +41,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/ccl/backupccl/backupencryption" "github.com/cockroachdb/cockroach/pkg/ccl/backupccl/backupinfo" "github.com/cockroachdb/cockroach/pkg/ccl/backupccl/backuppb" + "github.com/cockroachdb/cockroach/pkg/ccl/backupccl/backuputils" _ "github.com/cockroachdb/cockroach/pkg/ccl/kvccl" _ "github.com/cockroachdb/cockroach/pkg/ccl/multiregionccl" _ "github.com/cockroachdb/cockroach/pkg/ccl/multitenantccl" @@ -127,7 +128,7 @@ func TestBackupRestoreStatementResult(t *testing.T) { _, sqlDB, dir, cleanupFn := backupRestoreTestSetup(t, singleNode, numAccounts, InitManualReplication) defer cleanupFn() - if err := verifyBackupRestoreStatementResult( + if err := backuputils.VerifyBackupRestoreStatementResult( t, sqlDB, "BACKUP DATABASE data TO $1", localFoo, ); err != nil { t.Fatal(err) @@ -146,7 +147,7 @@ func TestBackupRestoreStatementResult(t *testing.T) { sqlDB.Exec(t, "CREATE DATABASE data2") - if err := verifyBackupRestoreStatementResult( + if err := backuputils.VerifyBackupRestoreStatementResult( t, sqlDB, "RESTORE data.* FROM $1 WITH OPTIONS (into_db='data2')", localFoo, ); err != nil { t.Fatal(err) diff --git a/pkg/ccl/backupccl/backuprand/BUILD.bazel b/pkg/ccl/backupccl/backuprand/BUILD.bazel new file mode 100644 index 000000000000..a4c913e05d4c --- /dev/null +++ b/pkg/ccl/backupccl/backuprand/BUILD.bazel @@ -0,0 +1,30 @@ +load("@io_bazel_rules_go//go:def.bzl", "go_test") + +go_test( + name = "backuprand_test", + srcs = [ + "backup_rand_test.go", + "main_test.go", + ], + deps = [ + "//pkg/base", + "//pkg/ccl", + "//pkg/ccl/backupccl/backuputils", + "//pkg/ccl/storageccl", + "//pkg/ccl/utilccl", + "//pkg/internal/sqlsmith", + "//pkg/security/securityassets", + "//pkg/security/securitytest", + "//pkg/server", + "//pkg/sql/randgen", + "//pkg/testutils", + "//pkg/testutils/serverutils", + "//pkg/testutils/skip", + "//pkg/testutils/sqlutils", + "//pkg/testutils/testcluster", + "//pkg/util/leaktest", + "//pkg/util/log", + "//pkg/util/randutil", + "@com_github_stretchr_testify//assert", + ], +) diff --git a/pkg/ccl/backupccl/backup_rand_test.go b/pkg/ccl/backupccl/backuprand/backup_rand_test.go similarity index 74% rename from pkg/ccl/backupccl/backup_rand_test.go rename to pkg/ccl/backupccl/backuprand/backup_rand_test.go index 71fa0a7dea6b..7308a24c3355 100644 --- a/pkg/ccl/backupccl/backup_rand_test.go +++ b/pkg/ccl/backupccl/backuprand/backup_rand_test.go @@ -6,7 +6,7 @@ // // https://github.com/cockroachdb/cockroach/blob/master/licenses/CCL.txt -package backupccl +package backuprand import ( "context" @@ -15,7 +15,10 @@ import ( "testing" "github.com/cockroachdb/cockroach/pkg/base" + _ "github.com/cockroachdb/cockroach/pkg/ccl" + "github.com/cockroachdb/cockroach/pkg/ccl/backupccl/backuputils" "github.com/cockroachdb/cockroach/pkg/internal/sqlsmith" + "github.com/cockroachdb/cockroach/pkg/sql/randgen" "github.com/cockroachdb/cockroach/pkg/testutils" "github.com/cockroachdb/cockroach/pkg/testutils/skip" "github.com/cockroachdb/cockroach/pkg/testutils/sqlutils" @@ -26,10 +29,10 @@ import ( "github.com/stretchr/testify/assert" ) -// TestBackupRestoreRandomDataRoundtrips creates random tables using SQLSmith -// and backs up and restores them, ensuring that the schema is properly -// preserved across the roundtrip. It tests that full database backup as well -// as all subsets of per-table backup roundtrip properly. +// TestBackupRestoreRandomDataRoundtrips conducts backup/restore roundtrips on +// randomly generated tables and verifies their data and schema are preserved. +// It tests that full database backup as well as all subsets of per-table backup +// roundtrip properly. func TestBackupRestoreRandomDataRoundtrips(t *testing.T) { defer leaktest.AfterTest(t)() defer log.Scope(t).Close(t) @@ -46,10 +49,12 @@ func TestBackupRestoreRandomDataRoundtrips(t *testing.T) { ExternalIODir: dir, }, } + const localFoo = "nodelocal://0/foo/" + ctx := context.Background() - tc := testcluster.StartTestCluster(t, singleNode, params) + tc := testcluster.StartTestCluster(t, 1, params) defer tc.Stopper().Stop(ctx) - InitManualReplication(tc) + tc.ToggleReplicateQueues(false) sqlDB := sqlutils.MakeSQLRunner(tc.Conns[0]) sqlDB.Exec(t, "CREATE DATABASE rand") @@ -59,6 +64,7 @@ func TestBackupRestoreRandomDataRoundtrips(t *testing.T) { t.Fatal(err) } } + numInserts := 20 tables := sqlDB.Query(t, `SELECT name FROM crdb_internal.tables WHERE database_name = 'rand' AND schema_name = 'public'`) @@ -68,38 +74,34 @@ database_name = 'rand' AND schema_name = 'public'`) if err := tables.Scan(&tableName); err != nil { t.Fatal(err) } + // Note: we do not care how many rows successfully populate + // the given table + if _, err := randgen.PopulateTableWithRandData(rng, tc.Conns[0], tableName, + numInserts); err != nil { + t.Fatal(err) + } tableNames = append(tableNames, tableName) } - getCreateStatementForTable := func(tableName string) string { - var ignored, createStatement string - table := sqlDB.QueryRow(t, fmt.Sprintf("SHOW CREATE TABLE %s", tableName)) - table.Scan(&ignored, &createStatement) - return createStatement - } - expectedCreateTableStmt := make(map[string]string) + expectedData := make(map[string][][]string) for _, tableName := range tableNames { - createStatement := getCreateStatementForTable(fmt.Sprintf("rand.%s", tableName)) - expectedCreateTableStmt[tableName] = createStatement + expectedCreateTableStmt[tableName] = sqlDB.QueryStr(t, fmt.Sprintf(`SELECT create_statement FROM [SHOW CREATE TABLE %s]`, tableName))[0][0] + expectedData[tableName] = sqlDB.QueryStr(t, fmt.Sprintf(`SELECT * FROM %s`, tableName)) } - // TODO(jordan): we should insert random data using SQLSmith mutation - // statements here. - // Now that we've created our random tables, backup and restore the whole DB // and compare all table descriptors for equality. dbBackup := localFoo + "wholedb" tablesBackup := localFoo + "alltables" dbBackups := []string{dbBackup, tablesBackup} - - if err := verifyBackupRestoreStatementResult( + if err := backuputils.VerifyBackupRestoreStatementResult( t, sqlDB, "BACKUP DATABASE rand TO $1", dbBackup, ); err != nil { t.Fatal(err) } - if err := verifyBackupRestoreStatementResult( + if err := backuputils.VerifyBackupRestoreStatementResult( t, sqlDB, "BACKUP TABLE rand.* TO $1", tablesBackup, ); err != nil { t.Fatal(err) @@ -111,8 +113,12 @@ database_name = 'rand' AND schema_name = 'public'`) verifyTables := func(t *testing.T, tableNames []string) { for _, tableName := range tableNames { t.Logf("Verifying table %s", tableName) - createStatement := getCreateStatementForTable("restoredb." + tableName) - assert.Equal(t, expectedCreateTableStmt[tableName], createStatement, "SHOW CREATE %s not equal after RESTORE", tableName) + restoreTable := "restoredb." + tableName + createStmt := sqlDB.QueryStr(t, + fmt.Sprintf(`SELECT create_statement FROM [SHOW CREATE TABLE %s]`, restoreTable))[0][0] + assert.Equal(t, expectedCreateTableStmt[tableName], createStmt, + "SHOW CREATE %s not equal after RESTORE", tableName) + sqlDB.CheckQueryResults(t, fmt.Sprintf(`SELECT * FROM %s`, tableName), expectedData[tableName]) } } @@ -122,7 +128,7 @@ database_name = 'rand' AND schema_name = 'public'`) for _, backup := range dbBackups { sqlDB.Exec(t, "DROP DATABASE IF EXISTS restoredb") sqlDB.Exec(t, "CREATE DATABASE restoredb") - if err := verifyBackupRestoreStatementResult( + if err := backuputils.VerifyBackupRestoreStatementResult( t, sqlDB, "RESTORE rand.* FROM $1 WITH OPTIONS (into_db='restoredb')", backup, ); err != nil { t.Fatal(err) @@ -130,7 +136,7 @@ database_name = 'rand' AND schema_name = 'public'`) verifyTables(t, tableNames) sqlDB.Exec(t, "DROP DATABASE IF EXISTS restoredb") - if err := verifyBackupRestoreStatementResult( + if err := backuputils.VerifyBackupRestoreStatementResult( t, sqlDB, "RESTORE DATABASE rand FROM $1 WITH OPTIONS (new_db_name='restoredb')", backup, ); err != nil { t.Fatal(err) diff --git a/pkg/ccl/backupccl/backuprand/main_test.go b/pkg/ccl/backupccl/backuprand/main_test.go new file mode 100644 index 000000000000..0be826b3405b --- /dev/null +++ b/pkg/ccl/backupccl/backuprand/main_test.go @@ -0,0 +1,34 @@ +// Copyright 2021 The Cockroach Authors. +// +// Licensed as a CockroachDB Enterprise file under the Cockroach Community +// License (the "License"); you may not use this file except in compliance with +// the License. You may obtain a copy of the License at +// +// https://github.com/cockroachdb/cockroach/blob/master/licenses/CCL.txt + +package backuprand + +import ( + "os" + "testing" + + _ "github.com/cockroachdb/cockroach/pkg/ccl/storageccl" + "github.com/cockroachdb/cockroach/pkg/ccl/utilccl" + "github.com/cockroachdb/cockroach/pkg/security/securityassets" + "github.com/cockroachdb/cockroach/pkg/security/securitytest" + "github.com/cockroachdb/cockroach/pkg/server" + "github.com/cockroachdb/cockroach/pkg/testutils/serverutils" + "github.com/cockroachdb/cockroach/pkg/testutils/testcluster" + "github.com/cockroachdb/cockroach/pkg/util/randutil" +) + +func TestMain(m *testing.M) { + defer utilccl.TestingEnableEnterprise()() + securityassets.SetLoader(securitytest.EmbeddedAssets) + randutil.SeedForTests() + serverutils.InitTestServerFactory(server.TestServerFactory) + serverutils.InitTestClusterFactory(testcluster.TestClusterFactory) + os.Exit(m.Run()) +} + +//go:generate ../../../util/leaktest/add-leaktest.sh *_test.go diff --git a/pkg/ccl/backupccl/backuputils/BUILD.bazel b/pkg/ccl/backupccl/backuputils/BUILD.bazel index 43ef675079ae..c4a19e857850 100644 --- a/pkg/ccl/backupccl/backuputils/BUILD.bazel +++ b/pkg/ccl/backupccl/backuputils/BUILD.bazel @@ -17,6 +17,8 @@ go_library( "//pkg/testutils/testcluster", "//pkg/workload/bank", "//pkg/workload/workloadsql", + "@com_github_cockroachdb_errors//:errors", "@com_github_cockroachdb_logtags//:logtags", + "@com_github_kr_pretty//:pretty", ], ) diff --git a/pkg/ccl/backupccl/backuputils/testutils.go b/pkg/ccl/backupccl/backuputils/testutils.go index 398c2977496c..22c6ab5b3d38 100644 --- a/pkg/ccl/backupccl/backuputils/testutils.go +++ b/pkg/ccl/backupccl/backuputils/testutils.go @@ -11,6 +11,8 @@ package backuputils import ( "context" gosql "database/sql" + "reflect" + "strings" "testing" "github.com/cockroachdb/cockroach/pkg/base" @@ -20,7 +22,9 @@ import ( "github.com/cockroachdb/cockroach/pkg/testutils/testcluster" "github.com/cockroachdb/cockroach/pkg/workload/bank" "github.com/cockroachdb/cockroach/pkg/workload/workloadsql" + "github.com/cockroachdb/errors" "github.com/cockroachdb/logtags" + "github.com/kr/pretty" ) const ( @@ -114,3 +118,60 @@ func BackupRestoreTestSetup( ) (tc *testcluster.TestCluster, sqlDB *sqlutils.SQLRunner, tempDir string, cleanup func()) { return backupRestoreTestSetupWithParams(t, clusterSize, numAccounts, init, base.TestClusterArgs{}) } + +// VerifyBackupRestoreStatementResult conducts a Backup or Restore and verifies +// it was properly written to the jobs table +func VerifyBackupRestoreStatementResult( + t *testing.T, sqlDB *sqlutils.SQLRunner, query string, args ...interface{}, +) error { + t.Helper() + rows := sqlDB.Query(t, query, args...) + + columns, err := rows.Columns() + if err != nil { + return err + } + if e, a := columns, []string{ + "job_id", "status", "fraction_completed", "rows", "index_entries", "bytes", + }; !reflect.DeepEqual(e, a) { + return errors.Errorf("unexpected columns:\n%s", strings.Join(pretty.Diff(e, a), "\n")) + } + + type job struct { + id int64 + status string + fractionCompleted float32 + } + + var expectedJob job + var actualJob job + var unused int64 + + if !rows.Next() { + if err := rows.Err(); err != nil { + return err + } + return errors.New("zero rows in result") + } + if err := rows.Scan( + &actualJob.id, &actualJob.status, &actualJob.fractionCompleted, &unused, &unused, &unused, + ); err != nil { + return err + } + if rows.Next() { + return errors.New("more than one row in result") + } + + sqlDB.QueryRow(t, + `SELECT job_id, status, fraction_completed FROM crdb_internal.jobs WHERE job_id = $1`, actualJob.id, + ).Scan( + &expectedJob.id, &expectedJob.status, &expectedJob.fractionCompleted, + ) + + if e, a := expectedJob, actualJob; !reflect.DeepEqual(e, a) { + return errors.Errorf("result does not match system.jobs:\n%s", + strings.Join(pretty.Diff(e, a), "\n")) + } + + return nil +} diff --git a/pkg/ccl/backupccl/utils_test.go b/pkg/ccl/backupccl/utils_test.go index be4a00fc84f4..324ec5230087 100644 --- a/pkg/ccl/backupccl/utils_test.go +++ b/pkg/ccl/backupccl/utils_test.go @@ -20,7 +20,6 @@ import ( "net/url" "os" "path/filepath" - "reflect" "regexp" "strings" "testing" @@ -45,7 +44,6 @@ import ( "github.com/cockroachdb/cockroach/pkg/workload/workloadsql" "github.com/cockroachdb/errors" "github.com/cockroachdb/logtags" - "github.com/kr/pretty" "github.com/stretchr/testify/require" ) @@ -138,61 +136,6 @@ func backupRestoreTestSetupEmpty( return backupRestoreTestSetupEmptyWithParams(t, clusterSize, tempDir, init, params) } -func verifyBackupRestoreStatementResult( - t *testing.T, sqlDB *sqlutils.SQLRunner, query string, args ...interface{}, -) error { - t.Helper() - rows := sqlDB.Query(t, query, args...) - - columns, err := rows.Columns() - if err != nil { - return err - } - if e, a := columns, []string{ - "job_id", "status", "fraction_completed", "rows", "index_entries", "bytes", - }; !reflect.DeepEqual(e, a) { - return errors.Errorf("unexpected columns:\n%s", strings.Join(pretty.Diff(e, a), "\n")) - } - - type job struct { - id int64 - status string - fractionCompleted float32 - } - - var expectedJob job - var actualJob job - var unused int64 - - if !rows.Next() { - if err := rows.Err(); err != nil { - return err - } - return errors.New("zero rows in result") - } - if err := rows.Scan( - &actualJob.id, &actualJob.status, &actualJob.fractionCompleted, &unused, &unused, &unused, - ); err != nil { - return err - } - if rows.Next() { - return errors.New("more than one row in result") - } - - sqlDB.QueryRow(t, - `SELECT job_id, status, fraction_completed FROM crdb_internal.jobs WHERE job_id = $1`, actualJob.id, - ).Scan( - &expectedJob.id, &expectedJob.status, &expectedJob.fractionCompleted, - ) - - if e, a := expectedJob, actualJob; !reflect.DeepEqual(e, a) { - return errors.Errorf("result does not match system.jobs:\n%s", - strings.Join(pretty.Diff(e, a), "\n")) - } - - return nil -} - func backupRestoreTestSetupEmptyWithParams( t testing.TB, clusterSize int,