diff --git a/pkg/migration/migrations/migrate_span_configs_test.go b/pkg/migration/migrations/migrate_span_configs_test.go index 956052920fa..1b0658f5b68 100644 --- a/pkg/migration/migrations/migrate_span_configs_test.go +++ b/pkg/migration/migrations/migrate_span_configs_test.go @@ -21,7 +21,6 @@ import ( "github.com/cockroachdb/cockroach/pkg/server" "github.com/cockroachdb/cockroach/pkg/spanconfig" "github.com/cockroachdb/cockroach/pkg/testutils" - "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/util/leaktest" @@ -33,7 +32,6 @@ import ( // span config reconciliation attempt, blocking until it occurs. func TestEnsureSpanConfigReconciliation(t *testing.T) { defer leaktest.AfterTest(t)() - skip.WithIssue(t, 75849, "flaky test") defer log.Scope(t).Close(t) ctx := context.Background() @@ -99,6 +97,8 @@ func TestEnsureSpanConfigReconciliation(t *testing.T) { } } +// TestEnsureSpanConfigReconciliationMultiNode verifies that the span config +// reconciliation migration works in a multi-node setting. func TestEnsureSpanConfigReconciliationMultiNode(t *testing.T) { defer leaktest.AfterTest(t)() defer log.Scope(t).Close(t) diff --git a/pkg/spanconfig/spanconfigjob/BUILD.bazel b/pkg/spanconfig/spanconfigjob/BUILD.bazel index b9a31469875..995675888a5 100644 --- a/pkg/spanconfig/spanconfigjob/BUILD.bazel +++ b/pkg/spanconfig/spanconfigjob/BUILD.bazel @@ -15,6 +15,7 @@ go_library( "//pkg/util/hlc", "//pkg/util/log", "//pkg/util/retry", + "//pkg/util/syncutil", "//pkg/util/timeutil", "@com_github_cockroachdb_errors//:errors", ], diff --git a/pkg/spanconfig/spanconfigjob/job.go b/pkg/spanconfig/spanconfigjob/job.go index a8d277523de..5afc9025a0a 100644 --- a/pkg/spanconfig/spanconfigjob/job.go +++ b/pkg/spanconfig/spanconfigjob/job.go @@ -23,6 +23,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/util/hlc" "github.com/cockroachdb/cockroach/pkg/util/log" "github.com/cockroachdb/cockroach/pkg/util/retry" + "github.com/cockroachdb/cockroach/pkg/util/syncutil" "github.com/cockroachdb/cockroach/pkg/util/timeutil" "github.com/cockroachdb/errors" ) @@ -91,18 +92,25 @@ func (r *resumer) Resume(ctx context.Context, execCtxI interface{}) (jobErr erro // timestamp. settingValues := &execCtx.ExecCfg().Settings.SV - persistCheckpoints := util.Every(reconciliationJobCheckpointInterval.Get(settingValues)) + persistCheckpointsMu := struct { + syncutil.Mutex + util.EveryN + }{} + persistCheckpointsMu.EveryN = util.Every(reconciliationJobCheckpointInterval.Get(settingValues)) + reconciliationJobCheckpointInterval.SetOnChange(settingValues, func(ctx context.Context) { - persistCheckpoints = util.Every(reconciliationJobCheckpointInterval.Get(settingValues)) + persistCheckpointsMu.Lock() + defer persistCheckpointsMu.Unlock() + persistCheckpointsMu.EveryN = util.Every(reconciliationJobCheckpointInterval.Get(settingValues)) }) - shouldPersistCheckpoint := true + checkpointingDisabled := false shouldSkipRetry := false var onCheckpointInterceptor func() error if knobs := execCtx.ExecCfg().SpanConfigTestingKnobs; knobs != nil { if knobs.JobDisablePersistingCheckpoints { - shouldPersistCheckpoint = false + checkpointingDisabled = true } shouldSkipRetry = knobs.JobDisableInternalRetry onCheckpointInterceptor = knobs.JobOnCheckpointInterceptor @@ -125,10 +133,14 @@ func (r *resumer) Resume(ctx context.Context, execCtxI interface{}) (jobErr erro } } - if !shouldPersistCheckpoint { + if checkpointingDisabled { return nil } - if !persistCheckpoints.ShouldProcess(timeutil.Now()) { + + persistCheckpointsMu.Lock() + shouldPersistCheckpoint := persistCheckpointsMu.ShouldProcess(timeutil.Now()) + persistCheckpointsMu.Unlock() + if !shouldPersistCheckpoint { return nil }