sctest: ensure job succeeds when retried upon success

We found a bug recently whereby a schema change which was resumed after its
last stage would fail if it were resumed again. This could happen if the
process of moving the state machine in the jobs subsystem to succeeded fails
for whatever reason. This change ensures that we test the failure case.

Release note: None

pkg/sql/schemachanger/sctest/BUILD.bazel

@@ -36,7 +36,9 @@ go_library(
         "//pkg/testutils/serverutils",
         "//pkg/testutils/skip",
         "//pkg/testutils/sqlutils",
+        "//pkg/util/log",
         "//pkg/util/randutil",
+        "//pkg/util/syncutil",
         "@com_github_cockroachdb_cockroach_go_v2//crdb",
         "@com_github_cockroachdb_datadriven//:datadriven",
         "@com_github_cockroachdb_errors//:errors",

pkg/sql/schemachanger/sctest/end_to_end.go

@@ -44,7 +44,10 @@ import (
 	"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/util/log"
+	"github.com/cockroachdb/cockroach/pkg/util/syncutil"
 	"github.com/cockroachdb/datadriven"
+	"github.com/cockroachdb/errors"
 	"github.com/stretchr/testify/require"
 )
 
@@ -61,7 +64,7 @@ func SingleNodeCluster(t *testing.T, knobs *scexec.TestingKnobs) (*gosql.DB, fun
 		DisableDefaultTestTenant: true,
 		Knobs: base.TestingKnobs{
 			SQLDeclarativeSchemaChanger: knobs,
-			JobsTestingKnobs:            jobs.NewTestingKnobsWithShortIntervals(),
+			JobsTestingKnobs:            newJobsKnobs(),
 			SQLExecutor: &sql.ExecutorTestingKnobs{
 				UseTransactionalDescIDGenerator: true,
 			},
@@ -72,6 +75,39 @@ func SingleNodeCluster(t *testing.T, knobs *scexec.TestingKnobs) (*gosql.DB, fun
 	}
 }
 
+// newJobsKnobs constructs jobs.TestingKnobs for the end-to-end tests.
+func newJobsKnobs() *jobs.TestingKnobs {
+	jobKnobs := jobs.NewTestingKnobsWithShortIntervals()
+
+	// We want to force the process of marking the job as successful
+	// to fail sometimes. This will ensure that the schema change job
+	// is idempotent.
+	var injectedFailures = struct {
+		syncutil.Mutex
+		m map[jobspb.JobID]struct{}
+	}{
+		m: make(map[jobspb.JobID]struct{}),
+	}
+	jobKnobs.BeforeUpdate = func(orig, updated jobs.JobMetadata) error {
+		sc := orig.Payload.GetNewSchemaChange()
+		if sc == nil {
+			return nil
+		}
+		if orig.Status != jobs.StatusRunning || updated.Status != jobs.StatusSucceeded {
+			return nil
+		}
+		injectedFailures.Lock()
+		defer injectedFailures.Unlock()
+		if _, ok := injectedFailures.m[orig.ID]; !ok {
+			injectedFailures.m[orig.ID] = struct{}{}
+			log.Infof(context.Background(), "injecting failure while marking job succeeded")
+			return errors.New("injected failure when marking succeeded")
+		}
+		return nil
+	}
+	return jobKnobs
+}
+
 // EndToEndSideEffects is a data-driven test runner that executes DDL statements in the
 // declarative schema changer injected with test dependencies and compares the
 // accumulated side effects logs with expected results from the data-driven