diff --git a/pkg/ccl/sqlproxyccl/proxy_handler_test.go b/pkg/ccl/sqlproxyccl/proxy_handler_test.go index 6726359472c5..f0bb2241e219 100644 --- a/pkg/ccl/sqlproxyccl/proxy_handler_test.go +++ b/pkg/ccl/sqlproxyccl/proxy_handler_test.go @@ -2056,6 +2056,7 @@ func TestConnectionMigration(t *testing.T) { // Now attempt a transfer concurrently with requests. initSuccessCount := f.metrics.ConnMigrationSuccessCount.Count() + initErrorRecoverableCount := f.metrics.ConnMigrationErrorRecoverableCount.Count() subCtx, cancel := context.WithCancel(tCtx) defer cancel() @@ -2095,7 +2096,7 @@ func TestConnectionMigration(t *testing.T) { // Check metrics. require.True(t, f.metrics.ConnMigrationSuccessCount.Count() > initSuccessCount+4) - require.Equal(t, int64(0), f.metrics.ConnMigrationErrorRecoverableCount.Count()) + require.Equal(t, initErrorRecoverableCount, f.metrics.ConnMigrationErrorRecoverableCount.Count()) require.Equal(t, int64(0), f.metrics.ConnMigrationErrorFatalCount.Count()) validateMiscMetrics(t) @@ -2105,6 +2106,7 @@ func TestConnectionMigration(t *testing.T) { // transfers should not close the connection. t.Run("failed_transfers_with_tx", func(t *testing.T) { initSuccessCount := f.metrics.ConnMigrationSuccessCount.Count() + initErrorRecoverableCount := f.metrics.ConnMigrationErrorRecoverableCount.Count() initAddr := queryAddr(tCtx, t, db) err = crdb.ExecuteTx(tCtx, db, nil /* txopts */, func(tx *gosql.Tx) error { @@ -2139,7 +2141,8 @@ func TestConnectionMigration(t *testing.T) { // still be active. require.Nil(t, f.ctx.Err()) require.Equal(t, initSuccessCount, f.metrics.ConnMigrationSuccessCount.Count()) - require.Equal(t, int64(5), f.metrics.ConnMigrationErrorRecoverableCount.Count()) + require.Equal(t, initErrorRecoverableCount+5, + f.metrics.ConnMigrationErrorRecoverableCount.Count()) require.Equal(t, int64(0), f.metrics.ConnMigrationErrorFatalCount.Count()) // Once the transaction is closed, transfers should work. @@ -2147,7 +2150,7 @@ func TestConnectionMigration(t *testing.T) { require.NotEqual(t, initAddr, queryAddr(tCtx, t, db)) require.Nil(t, f.ctx.Err()) require.Equal(t, initSuccessCount+1, f.metrics.ConnMigrationSuccessCount.Count()) - require.Equal(t, int64(5), f.metrics.ConnMigrationErrorRecoverableCount.Count()) + require.True(t, f.metrics.ConnMigrationErrorRecoverableCount.Count() >= initErrorRecoverableCount+5) require.Equal(t, int64(0), f.metrics.ConnMigrationErrorFatalCount.Count()) validateMiscMetrics(t) diff --git a/pkg/jobs/utils.go b/pkg/jobs/utils.go index 17c39fee9724..367b52241b31 100644 --- a/pkg/jobs/utils.go +++ b/pkg/jobs/utils.go @@ -156,16 +156,24 @@ 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 @@ -173,15 +181,19 @@ func isJobTypeColumnDoesNotExistError(err error) bool { // 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 } diff --git a/pkg/upgrade/upgrademanager/BUILD.bazel b/pkg/upgrade/upgrademanager/BUILD.bazel index d1f2f7612ff3..5e46cec877d3 100644 --- a/pkg/upgrade/upgrademanager/BUILD.bazel +++ b/pkg/upgrade/upgrademanager/BUILD.bazel @@ -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", diff --git a/pkg/upgrade/upgrademanager/manager_external_test.go b/pkg/upgrade/upgrademanager/manager_external_test.go index f965243f8fc3..5cd231e9492f 100644 --- a/pkg/upgrade/upgrademanager/manager_external_test.go +++ b/pkg/upgrade/upgrademanager/manager_external_test.go @@ -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" @@ -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