Merge pull request #134865 from cockroachdb/blathers/backport-release…

…-24.3-134746

release-24.3: scjob: support handling permanent job errors

pkg/sql/schemachanger/schemachanger_test.go

@@ -922,3 +922,36 @@ func isPQErrWithCode(err error, codes ...pgcode.Code) bool {
 	}
 	return false
 }
+
+func TestSchemaChangerFailsOnMissingDesc(t *testing.T) {
+	defer leaktest.AfterTest(t)()
+	defer log.Scope(t).Close(t)
+
+	ctx := context.Background()
+	var params base.TestServerArgs
+	params.Knobs = base.TestingKnobs{
+		SQLDeclarativeSchemaChanger: &scexec.TestingKnobs{
+			AfterStage: func(p scplan.Plan, stageIdx int) error {
+				if p.Params.ExecutionPhase != scop.PostCommitPhase || stageIdx > 1 {
+					return nil
+				}
+
+				return catalog.ErrDescriptorNotFound
+			},
+		},
+		JobsTestingKnobs: jobs.NewTestingKnobsWithShortIntervals(),
+	}
+
+	s, sqlDB, _ := serverutils.StartServer(t, params)
+	defer s.Stopper().Stop(ctx)
+
+	tdb := sqlutils.MakeSQLRunner(sqlDB)
+	tdb.Exec(t, `SET use_declarative_schema_changer = 'off'`)
+	tdb.Exec(t, `CREATE DATABASE db`)
+	tdb.Exec(t, `CREATE TABLE db.t (a INT PRIMARY KEY)`)
+	tdb.Exec(t, `SET use_declarative_schema_changer = 'unsafe'`)
+	tdb.ExpectErr(t, "descriptor not found", `ALTER TABLE db.t ADD COLUMN b INT NOT NULL DEFAULT (123)`)
+	// Validate the job has hit a terminal state.
+	tdb.CheckQueryResults(t, "SELECT status FROM crdb_internal.jobs WHERE statement LIKE '%ADD COLUMN%'",
+		[][]string{{"failed"}})
+}

pkg/sql/schemachanger/scjob/job.go

@@ -62,6 +62,13 @@ func (n *newSchemaChangeResumer) OnFailOrCancel(
 ) error {
 	execCtx := execCtxI.(sql.JobExecContext)
 	execCfg := execCtx.ExecCfg()
+	// Permanent error has been hit, so there is no rollback
+	// from here. Only if the status is reverting will these be
+	// treated as fatal.
+	if jobs.IsPermanentJobError(err) && n.job.Status() == jobs.StatusReverting {
+		log.Warningf(ctx, "schema change will not rollback; permanent error detected: %v", err)
+		return nil
+	}
 	n.rollbackCause = err
 
 	// Clean up any protected timestamps as a last resort, in case the job
@@ -148,7 +155,7 @@ func (n *newSchemaChangeResumer) run(ctx context.Context, execCtxI interface{})
 		// permanent job error, so that non-cancelable jobs don't get retried. If a
 		// descriptor has gone missing, it isn't likely to come back.
 		if errors.IsAny(err, catalog.ErrDescriptorNotFound, catalog.ErrDescriptorDropped, catalog.ErrReferencedDescriptorNotFound) {
-			err = jobs.MarkAsPermanentJobError(err)
+			return jobs.MarkAsPermanentJobError(err)
 		}
 		return err
 	}