From dbd76c3d37feedd3064e16a4dd22cfbb853a4e27 Mon Sep 17 00:00:00 2001 From: Steven Danna Date: Thu, 14 Dec 2023 11:33:01 +0000 Subject: [PATCH] roachtestutil: don't reference context after cancellation Previously, CheckInvalidDescriptors accessed `rows` after RunWithTimeout already returned. This is a problem because the context passed to db.QueryContext is retained by the returned rows object. That context is in turn cancelled when RunWithTimeout returns. As a result, it is possible to see a context cancelled error when accessing rows if it observes the context cancellation quickly enough. Fixes #116426 Release note: None --- .../roachtestutil/validation_check.go | 23 +++++++------------ 1 file changed, 8 insertions(+), 15 deletions(-) diff --git a/pkg/cmd/roachtest/roachtestutil/validation_check.go b/pkg/cmd/roachtest/roachtestutil/validation_check.go index 31a9439766b7..49e3b6eb8884 100644 --- a/pkg/cmd/roachtest/roachtestutil/validation_check.go +++ b/pkg/cmd/roachtest/roachtestutil/validation_check.go @@ -67,29 +67,22 @@ WHERE t.status NOT IN ('RANGE_CONSISTENT', 'RANGE_INDETERMINATE')`) // CheckInvalidDescriptors returns an error if there exists any descriptors in // the crdb_internal.invalid_objects virtual table. func CheckInvalidDescriptors(ctx context.Context, db *gosql.DB) error { - // Because crdb_internal.invalid_objects is a virtual table, by default, the - // query will take a lease on the database sqlDB is connected to and only run - // the query on the given database. The "" prefix prevents this lease - // acquisition and allows the query to fetch all descriptors in the cluster. - var ( - rows *gosql.Rows - err error - ) + var invalidIDs string if err := timeutil.RunWithTimeout(ctx, "descriptor validation", time.Minute, func(ctx context.Context) error { - rows, err = db.QueryContext(ctx, ` -SELECT id, obj_name, error FROM "".crdb_internal.invalid_objects`) + // Because crdb_internal.invalid_objects is a virtual table, by default, the + // query will take a lease on the database sqlDB is connected to and only run + // the query on the given database. The "" prefix prevents this lease + // acquisition and allows the query to fetch all descriptors in the cluster. + rows, err := db.QueryContext(ctx, `SELECT id, obj_name, error FROM "".crdb_internal.invalid_objects`) if err != nil { return err } - return nil + invalidIDs, err = sqlutils.RowsToDataDrivenOutput(rows) + return err }); err != nil { return err } - invalidIDs, err := sqlutils.RowsToDataDrivenOutput(rows) - if err != nil { - return err - } if invalidIDs != "" { return errors.Errorf("the following descriptor ids are invalid\n%v", invalidIDs) }