Skip to content

Commit

Permalink
backupccl: de-flake TestPaginatedBackupTenant
Browse files Browse the repository at this point in the history
This test readily fails under stress with span configs enabled (cockroachdb#73876).
Now that we split within the tenant keyspace for tenant tables, the
export requests generated for each BACKUP is liable to be retried if
concurrent with range splits. The splits causes KV to error out with
RangeKeyMismatchErrors, at which point the request is internally
retried. The test, which wants to assert on what ExportSpan requests are
intercepted, previously did not need to consider these retries/need to
de-dup -- it now does.

Release note: None
  • Loading branch information
irfansharif committed Jan 18, 2022
1 parent 365b4da commit 04fb64e
Showing 1 changed file with 13 additions and 11 deletions.
24 changes: 13 additions & 11 deletions pkg/ccl/backupccl/backup_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -767,7 +767,7 @@ func TestBackupRestoreAppend(t *testing.T) {

sqlDB.ExpectErr(t, "cannot append a backup of specific", "BACKUP system.users TO ($1, $2, "+
"$3)", test.backups...)
//TODO(dt): prevent backing up different targets to same collection?
// TODO(dt): prevent backing up different targets to same collection?

sqlDB.Exec(t, "DROP DATABASE data CASCADE")
sqlDB.Exec(t, "RESTORE DATABASE data FROM ($1, $2, $3)", test.backups...)
Expand Down Expand Up @@ -6793,6 +6793,8 @@ func TestPaginatedBackupTenant(t *testing.T) {
serverArgs := base.TestServerArgs{Knobs: base.TestingKnobs{JobsTestingKnobs: jobs.NewTestingKnobsWithShortIntervals()}}
params := base.TestClusterArgs{ServerArgs: serverArgs}
var numExportRequests int

exportRequestSpansSet := make(map[string]struct{})
exportRequestSpans := make([]string, 0)

requestSpanStr := func(span roachpb.Span, timestamp hlc.Timestamp) string {
Expand All @@ -6816,10 +6818,12 @@ func TestPaginatedBackupTenant(t *testing.T) {
for _, ru := range request.Requests {
if exportRequest, ok := ru.GetInner().(*roachpb.ExportRequest); ok &&
!isLeasingExportRequest(exportRequest) {
exportRequestSpans = append(
exportRequestSpans,
requestSpanStr(roachpb.Span{Key: exportRequest.Key, EndKey: exportRequest.EndKey}, exportRequest.ResumeKeyTS),
)
req := requestSpanStr(roachpb.Span{Key: exportRequest.Key, EndKey: exportRequest.EndKey}, exportRequest.ResumeKeyTS)
if _, found := exportRequestSpansSet[req]; found {
return nil // nothing to do
}
exportRequestSpansSet[req] = struct{}{}
exportRequestSpans = append(exportRequestSpans, req)
numExportRequests++
}
}
Expand Down Expand Up @@ -6847,6 +6851,7 @@ func TestPaginatedBackupTenant(t *testing.T) {

resetStateVars := func() {
numExportRequests = 0
exportRequestSpansSet = make(map[string]struct{})
exportRequestSpans = exportRequestSpans[:0]
}

Expand All @@ -6863,26 +6868,23 @@ func TestPaginatedBackupTenant(t *testing.T) {
systemDB.Exec(t, `SET CLUSTER SETTING kv.bulk_sst.max_allowed_overage='0b'`)

tenant10.Exec(t, `BACKUP DATABASE foo TO 'userfile://defaultdb.myfililes/test'`)
require.Equal(t, 1, numExportRequests)
startingSpan := roachpb.Span{Key: []byte("/Tenant/10/Table/56/1"), EndKey: []byte("/Tenant/10/Table/56/2")}
require.Equal(t, exportRequestSpans, []string{startingSpan.String()})
require.Equal(t, []string{startingSpan.String()}, exportRequestSpans)
resetStateVars()

// Two ExportRequests with one resume span.
systemDB.Exec(t, `SET CLUSTER SETTING kv.bulk_sst.target_size='50b'`)
tenant10.Exec(t, `BACKUP DATABASE foo TO 'userfile://defaultdb.myfililes/test2'`)
require.Equal(t, 2, numExportRequests)
startingSpan = roachpb.Span{Key: []byte("/Tenant/10/Table/56/1"),
EndKey: []byte("/Tenant/10/Table/56/2")}
resumeSpan := roachpb.Span{Key: []byte("/Tenant/10/Table/56/1/510/0"),
EndKey: []byte("/Tenant/10/Table/56/2")}
require.Equal(t, exportRequestSpans, []string{startingSpan.String(), resumeSpan.String()})
require.Equal(t, []string{startingSpan.String(), resumeSpan.String()}, exportRequestSpans)
resetStateVars()

// One ExportRequest for every KV.
systemDB.Exec(t, `SET CLUSTER SETTING kv.bulk_sst.target_size='10b'`)
tenant10.Exec(t, `BACKUP DATABASE foo TO 'userfile://defaultdb.myfililes/test3'`)
require.Equal(t, 5, numExportRequests)
var expected []string
for _, resume := range []exportResumePoint{
{[]byte("/Tenant/10/Table/56/1"), []byte("/Tenant/10/Table/56/2"), withoutTS},
Expand Down Expand Up @@ -8885,7 +8887,7 @@ DROP TABLE foo;
}

// TestRestoreNewDatabaseName tests the new_db_name optional feature for single database
//restores, which allows the user to rename the database they intend to restore.
// restores, which allows the user to rename the database they intend to restore.
func TestRestoreNewDatabaseName(t *testing.T) {
defer leaktest.AfterTest(t)()
defer log.Scope(t).Close(t)
Expand Down

0 comments on commit 04fb64e

Please sign in to comment.