backupccl: add tenant backup nemesis tests by stevendanna · Pull Request #92327 · cockroachdb/cockroach

stevendanna · 2022-11-22T12:46:29Z

This adds a bit of randomized testing to stress incremental tenant backups.

Tenant backups were previously untenable because of non-MVCC operations. Now that we support MVCC-compatible bulk operations, they should in principal be possible.

Epic: CRDB-20264

Release note: None

cockroach-teamcity · 2022-11-22T12:46:40Z

This change is

stevendanna · 2022-12-21T12:40:07Z

@msbutler Friendly ping here. I think this should be RFAL now that we've fixed some of the underlying issues that were preventing it from working before.

msbutler

This looks great! I'm having a hard time understanding/reviewing the new nemesis infra. I'll give it another pass once you add some comments, or we can setup a time to chat if that's better.

msbutler · 2022-12-21T15:21:26Z

+	})
+
+	require.NoError(t, g.Wait())
+	restoreQuery := fmt.Sprintf("RESTORE TENANT 10 FROM LATEST IN '%s' AS OF SYSTEM TIME %s WITH tenant = '11'", backupLoc, aost)


seems like we only run validation on the latest aost set in the above goroutine. What if we randomly chose an aost so that we restore at some random point in the chain?

I'm going to leave this as a follow up. To do this, we need to track which tables are valid to test at the point of the restore.

Perhaps we could get around that by just fingerprinting the whole tenant.

Sounds good. We could use Aditya's fingerprint job here and remove the TablesToCheck stuff.

The reason table-by-table is nice for now is that the table names are directly related to the nemesis that runs which gives you a foothold into what failed.

ah, good point. This also makes me wonder: should we print or log the seed returned by the NewPseudoRand() call to make it easier to repro a future test failure?

msbutler · 2022-12-21T15:37:39Z

+		validateTableAcrossTenantRestore(t, tenant10SQLDB, tenant11SQLDB, fmt.Sprintf(`bank."%s"`, name), aost)
+	}
+}
+


these helper functions are quite nice, I wonder if it's worth putting them in the backup utils_test.go package. Happy to leave this as a todo.

msbutler · 2022-12-21T18:00:23Z

+
+		nemeses: []nemesis{
+			{name: "CANCELED IMPORT INTO",
+				impl: func(ctx context.Context, n *randomBackupNemesis, db *gosql.DB) error {


the number of incremental backups the import job spanned had some affect on the corruption cases we saw. I wonder if we could randomly vary the speed of the import job, or any of the jobs below.

This seems like it would be a fun thing to add. The other thing I was thinking of adding to the "listener" stuff was something where the operations could let you know when an "interesting" point in the operations was occurring and then allow you to block the operation, take a backup, and continue.

either would be great. whatever is easier to implement!

Well, this would require a good deal more infrastructure such as that which is already causing confusion. So I'll leave this to whoever is motivated to touch this test next.

stevendanna · 2022-12-22T18:22:28Z

@msbutler I left a few new comments. All of the "oneTimeListener" stuff is there because I was playing around with whether forcing the nemesis operations to be interleaved with the incrementals was important. In the end I decided to just bump the incremental count to 30 and let randomness take its course, but since it is just a few lines of code I've left it to keep playing with as we add more operations.

stevendanna · 2022-12-22T22:01:31Z

Okie Dokie @msbutler I went ahead and removed the RequireStart stuff and confirmed that this still catches the import bug with range tombstones turned off.

msbutler

LGTM! Thanks for simplifying the test a bit!

msbutler · 2022-12-23T16:26:41Z

And good call to revert the extra infra in a separate commit -- it will make it easy to come back to.

This adds a bit of randomized testing to stress incremental tenant backups. Tenant backups were previously untenable because of non-MVCC operations. Now that we support MVCC-compatible bulk operations, they should in principal be possible. Release note: None

Epic: none Release note: None

stevendanna · 2023-01-03T15:25:01Z

bors r=msbutler

craig · 2023-01-03T15:39:24Z

Build failed (retrying...):

Bazel Essential CI (Cockroach)

craig · 2023-01-03T16:30:39Z

Build succeeded:

Bazel Essential CI (Cockroach)

stevendanna requested review from a team as code owners November 22, 2022 12:46

stevendanna requested review from herkolategan, rhu713 and smg260 and removed request for a team November 22, 2022 12:46

stevendanna requested review from dt and msbutler and removed request for herkolategan, rhu713 and smg260 November 23, 2022 14:32

stevendanna commented Nov 23, 2022

View reviewed changes

Comment thread pkg/ccl/backupccl/tenant_backup_nemesis_test.go Outdated

stevendanna force-pushed the ssd/tenant-backup-nemesis branch from 2ade0ee to d08ac44 Compare November 30, 2022 15:04

stevendanna force-pushed the ssd/tenant-backup-nemesis branch from d08ac44 to 7b4795e Compare December 21, 2022 12:38

stevendanna force-pushed the ssd/tenant-backup-nemesis branch from 7b4795e to 1f70a1d Compare December 21, 2022 14:35

msbutler reviewed Dec 21, 2022

View reviewed changes

stevendanna force-pushed the ssd/tenant-backup-nemesis branch 2 times, most recently from 6b681ef to f0a00d9 Compare December 22, 2022 18:20

stevendanna force-pushed the ssd/tenant-backup-nemesis branch from f0a00d9 to 75d348f Compare December 22, 2022 22:01

msbutler approved these changes Dec 23, 2022

View reviewed changes

stevendanna added 2 commits January 3, 2023 09:55

backupccl: remove unnecessary infra from TestTenantBackupNemesis

4cc6007

Epic: none Release note: None

stevendanna force-pushed the ssd/tenant-backup-nemesis branch from 75d348f to 4cc6007 Compare January 3, 2023 10:00

craig bot merged commit be9e8fb into cockroachdb:master Jan 3, 2023

Conversation

stevendanna commented Nov 22, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cockroach-teamcity commented Nov 22, 2022

Uh oh!

Uh oh!

stevendanna commented Dec 21, 2022

Uh oh!

msbutler left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

msbutler Dec 21, 2022

Choose a reason for hiding this comment

Uh oh!

stevendanna Dec 22, 2022

Choose a reason for hiding this comment

Uh oh!

msbutler Dec 22, 2022

Choose a reason for hiding this comment

Uh oh!

stevendanna Dec 22, 2022

Choose a reason for hiding this comment

Uh oh!

msbutler Dec 23, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

msbutler Dec 21, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

msbutler Dec 21, 2022

Choose a reason for hiding this comment

Uh oh!

stevendanna Dec 22, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

msbutler Dec 22, 2022

Choose a reason for hiding this comment

Uh oh!

stevendanna Dec 22, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stevendanna commented Dec 22, 2022

Uh oh!

stevendanna commented Dec 22, 2022

Uh oh!

msbutler left a comment

Choose a reason for hiding this comment

Uh oh!

msbutler commented Dec 23, 2022

Uh oh!

stevendanna commented Jan 3, 2023

Uh oh!

craig bot commented Jan 3, 2023

Uh oh!

craig bot commented Jan 3, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

stevendanna commented Nov 22, 2022 •

edited

Loading

msbutler left a comment •

edited

Loading

msbutler Dec 23, 2022 •

edited

Loading

stevendanna Dec 22, 2022 •

edited

Loading

stevendanna Dec 22, 2022 •

edited

Loading