Skip to content

Commit 9cd15a2

Browse files
committed
upgrademanager: unskip TestMigrationFailure
The test no longer fails with our change in Fixes: cockroachdb#106648 Fixes: cockroachdb#106762 Release note: None
1 parent 7a597b6 commit 9cd15a2

File tree

3 files changed

+19
-11
lines changed

3 files changed

+19
-11
lines changed

pkg/jobs/utils.go

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -156,32 +156,44 @@ func JobExists(
156156
return row != nil, nil
157157
}
158158

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

166+
// isJobInfoTableDoesNotExistError returns true if the error is of the form
167+
// `related "job_info" does not exist`.
168+
func isJobInfoTableDoesNotExistError(err error) bool {
169+
return pgerror.GetPGCode(err) == pgcode.UndefinedTable &&
170+
strings.Contains(err.Error(), "relation \"system.job_info\" does not exist")
171+
}
172+
166173
// MaybeGenerateForcedRetryableError returns a
167174
// TransactionRetryWithProtoRefreshError that will cause the txn to be retried
168-
// if the error is because of an undefined job_type column.
175+
// if the error is because of an undefined job_type column or missing job_info
176+
// table.
169177
//
170178
// In https://github.com/cockroachdb/cockroach/issues/106762 we noticed that if
171179
// a query is executed with an AS OF SYSTEM TIME clause that picks a transaction
172180
// timestamp before the job_type migration, then parts of the jobs
173181
// infrastructure will attempt to query the job_type column even though it
174182
// doesn't exist at the transaction's timestamp.
175183
//
176-
// As a short term fix, when we encounter an `UndefinedColumn` error we
177-
// generate a synthetic retryable error so that the txn is pushed to a
178-
// higher timestamp at which the upgrade will have completed and the
179-
// `job_type` column will be visible. The longer term fix is being tracked
180-
// in https://github.com/cockroachdb/cockroach/issues/106764.
184+
// As a short term fix, when we encounter an `UndefinedTable` or
185+
// `UndefinedColumn` error we generate a synthetic retryable error so that the
186+
// txn is pushed to a higher timestamp at which the upgrade will have completed
187+
// and the table/column will be visible. The longer term fix is being tracked in
188+
// https://github.com/cockroachdb/cockroach/issues/106764.
181189
func MaybeGenerateForcedRetryableError(ctx context.Context, txn *kv.Txn, err error) error {
182190
if err != nil && isJobTypeColumnDoesNotExistError(err) {
183191
return txn.GenerateForcedRetryableError(ctx, "synthetic error "+
184192
"to push timestamp to after the `job_type` upgrade has run")
185193
}
194+
if err != nil && isJobInfoTableDoesNotExistError(err) {
195+
return txn.GenerateForcedRetryableError(ctx, "synthetic error "+
196+
"to push timestamp to after the `job_info` upgrade has run")
197+
}
186198
return err
187199
}

pkg/upgrade/upgrademanager/BUILD.bazel

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,6 @@ go_test(
6868
"//pkg/sql/sem/eval",
6969
"//pkg/testutils",
7070
"//pkg/testutils/serverutils",
71-
"//pkg/testutils/skip",
7271
"//pkg/testutils/sqlutils",
7372
"//pkg/testutils/testcluster",
7473
"//pkg/upgrade",

pkg/upgrade/upgrademanager/manager_external_test.go

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,6 @@ import (
3838
"github.com/cockroachdb/cockroach/pkg/sql/sem/eval"
3939
"github.com/cockroachdb/cockroach/pkg/testutils"
4040
"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
41-
"github.com/cockroachdb/cockroach/pkg/testutils/skip"
4241
"github.com/cockroachdb/cockroach/pkg/testutils/sqlutils"
4342
"github.com/cockroachdb/cockroach/pkg/testutils/testcluster"
4443
"github.com/cockroachdb/cockroach/pkg/upgrade"
@@ -683,8 +682,6 @@ func TestMigrationFailure(t *testing.T) {
683682
defer leaktest.AfterTest(t)()
684683
defer log.Scope(t).Close(t)
685684

686-
skip.WithIssue(t, 106648)
687-
688685
ctx := context.Background()
689686

690687
// Configure the range of versions used by the test

0 commit comments

Comments
 (0)