Skip to content

Commit

Permalink
sql: fix TestValidationWithProtectedTS flake
Browse files Browse the repository at this point in the history
Previously, the test TestValidationWithProtectedTS would hang if a Go
routine would hit an error, since a channel would not be posted on a
error. It would also not gracefully handle TS after replica GC errors on
transactions, which could happen on overloaded machines. To address this
patch makes the test more resilient to the Go routine hitting errors so
that it doesn't time out, and adds retry logic for TS after replica GC
errors.

Fixes: cockroachdb#123447
Release note: None
  • Loading branch information
fqazi committed May 7, 2024
1 parent 22b4a2c commit ff99247
Showing 1 changed file with 20 additions and 5 deletions.
25 changes: 20 additions & 5 deletions pkg/sql/backfill_protected_timestamp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,11 @@ func TestValidationWithProtectedTS(t *testing.T) {
grp := ctxgroup.WithContext(ctx)
grp.Go(func() error {
<-indexValidationQueryWait
defer func() {
// Always unblock the validation query, even if something
// fails here.
indexValidationQueryResume <- struct{}{}
}()
getTableRangeIDs := func(t *testing.T) ([]int64, error) {
t.Helper()
rows, err := db.QueryContext(ctx, "WITH r AS (SHOW RANGES FROM TABLE t) SELECT range_id FROM r ORDER BY start_key")
Expand All @@ -160,6 +165,7 @@ func TestValidationWithProtectedTS(t *testing.T) {
return err
}
const retryTxnErrorSubstring = "restart transaction"
const replicaGCError = "must be after replica GC threshold"
for {
if _, err := db.ExecContext(ctx, "BEGIN"); err != nil {
return err
Expand All @@ -175,7 +181,8 @@ func TestValidationWithProtectedTS(t *testing.T) {
}
_, err = db.ExecContext(ctx, "COMMIT")
if err != nil {
if strings.Contains(err.Error(), retryTxnErrorSubstring) {
if strings.Contains(err.Error(), retryTxnErrorSubstring) ||
strings.Contains(err.Error(), replicaGCError) {
err = nil
continue
}
Expand All @@ -190,7 +197,6 @@ func TestValidationWithProtectedTS(t *testing.T) {
return err
}
}
indexValidationQueryResume <- struct{}{}
return nil
})
grp.Go(func() error {
Expand All @@ -199,12 +205,21 @@ func TestValidationWithProtectedTS(t *testing.T) {
})

require.NoError(t, grp.Wait())
// Validate the rows were removed due to the drop
res := r.QueryStr(t, `SELECT n FROM t@foo`)
// Validate the rows were removed due to the delete.
// Note: Because of the low GC timestamp we can hit "must be after replica GC
// threshold" errors.
var res [][]string
testutils.SucceedsSoon(t, func() error {
rows, err := r.DB.QueryContext(ctx, `SELECT n FROM t@foo`)
if err != nil {
return err
}
res, err = sqlutils.RowsToStrMatrix(rows)
return err
})
if len(res) != 1 {
t.Errorf("expected %d entries, got %d", 1, len(res))
}
require.NoError(t, db.Close())
}

// TestBackfillQueryWithProtectedTS backfills a query into a table and confirms
Expand Down

0 comments on commit ff99247

Please sign in to comment.