Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

upgrademanager: unskip TestMigrationFailure #108029

Merged
merged 1 commit into from
Aug 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 19 additions & 7 deletions pkg/jobs/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -156,32 +156,44 @@ func JobExists(
return row != nil, nil
}

// IsJobTypeColumnDoesNotExistError returns true if the error is of the form
// isJobTypeColumnDoesNotExistError returns true if the error is of the form
// `column "job_type" does not exist`.
func isJobTypeColumnDoesNotExistError(err error) bool {
return pgerror.GetPGCode(err) == pgcode.UndefinedColumn &&
strings.Contains(err.Error(), "column \"job_type\" does not exist")
}

// isJobInfoTableDoesNotExistError returns true if the error is of the form
// `related "job_info" does not exist`.
func isJobInfoTableDoesNotExistError(err error) bool {
return pgerror.GetPGCode(err) == pgcode.UndefinedTable &&
strings.Contains(err.Error(), "relation \"system.job_info\" does not exist")
}

// MaybeGenerateForcedRetryableError returns a
// TransactionRetryWithProtoRefreshError that will cause the txn to be retried
// if the error is because of an undefined job_type column.
// if the error is because of an undefined job_type column or missing job_info
// table.
//
// In https://github.com/cockroachdb/cockroach/issues/106762 we noticed that if
// a query is executed with an AS OF SYSTEM TIME clause that picks a transaction
// timestamp before the job_type migration, then parts of the jobs
// infrastructure will attempt to query the job_type column even though it
// doesn't exist at the transaction's timestamp.
//
// As a short term fix, when we encounter an `UndefinedColumn` error we
// generate a synthetic retryable error so that the txn is pushed to a
// higher timestamp at which the upgrade will have completed and the
// `job_type` column will be visible. The longer term fix is being tracked
// in https://github.com/cockroachdb/cockroach/issues/106764.
// As a short term fix, when we encounter an `UndefinedTable` or
// `UndefinedColumn` error we generate a synthetic retryable error so that the
// txn is pushed to a higher timestamp at which the upgrade will have completed
// and the table/column will be visible. The longer term fix is being tracked in
// https://github.com/cockroachdb/cockroach/issues/106764.
func MaybeGenerateForcedRetryableError(ctx context.Context, txn *kv.Txn, err error) error {
if err != nil && isJobTypeColumnDoesNotExistError(err) {
return txn.GenerateForcedRetryableError(ctx, "synthetic error "+
"to push timestamp to after the `job_type` upgrade has run")
}
if err != nil && isJobInfoTableDoesNotExistError(err) {
return txn.GenerateForcedRetryableError(ctx, "synthetic error "+
"to push timestamp to after the `job_info` upgrade has run")
}
return err
}
1 change: 0 additions & 1 deletion pkg/upgrade/upgrademanager/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,6 @@ go_test(
"//pkg/sql/sem/eval",
"//pkg/testutils",
"//pkg/testutils/serverutils",
"//pkg/testutils/skip",
"//pkg/testutils/sqlutils",
"//pkg/testutils/testcluster",
"//pkg/upgrade",
Expand Down
3 changes: 0 additions & 3 deletions pkg/upgrade/upgrademanager/manager_external_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ import (
"github.com/cockroachdb/cockroach/pkg/sql/sem/eval"
"github.com/cockroachdb/cockroach/pkg/testutils"
"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
"github.com/cockroachdb/cockroach/pkg/testutils/skip"
"github.com/cockroachdb/cockroach/pkg/testutils/sqlutils"
"github.com/cockroachdb/cockroach/pkg/testutils/testcluster"
"github.com/cockroachdb/cockroach/pkg/upgrade"
Expand Down Expand Up @@ -683,8 +682,6 @@ func TestMigrationFailure(t *testing.T) {
defer leaktest.AfterTest(t)()
defer log.Scope(t).Close(t)

skip.WithIssue(t, 106648)

ctx := context.Background()

// Configure the range of versions used by the test
Expand Down