diff --git a/pkg/move/check/check.go b/pkg/move/check/check.go index 86c7374fe..d7a3fbb3f 100644 --- a/pkg/move/check/check.go +++ b/pkg/move/check/check.go @@ -6,6 +6,7 @@ import ( "context" "database/sql" "log/slog" + "slices" "sync" "github.com/block/spirit/pkg/applier" @@ -72,8 +73,19 @@ func registerCheck(name string, callback func(context.Context, Resources, *slog. // RunChecks runs all checks that are registered for the given scope func RunChecks(ctx context.Context, r Resources, logger *slog.Logger, scope ScopeFlag) error { - for _, check := range checks { - if check.scope != scope { + return RunChecksExcluding(ctx, r, logger, scope) +} + +// RunChecksExcluding runs all checks registered for the given scope except +// those named in exclude. The runner's --force recovery path uses it to +// re-run the post-setup checks minus the target-state check before wiping +// the target: wiping only cures target-side state, so a failure in any other +// (source-side) check must surface before the target is destroyed. New +// checks are deliberately included by default — excluding too little only +// blocks a wipe, excluding too much could green-light one. +func RunChecksExcluding(ctx context.Context, r Resources, logger *slog.Logger, scope ScopeFlag, exclude ...string) error { + for name, check := range checks { + if check.scope != scope || slices.Contains(exclude, name) { continue } err := check.callback(ctx, r, logger) diff --git a/pkg/move/check/target_state.go b/pkg/move/check/target_state.go index bef4f3a2e..b4bbe814b 100644 --- a/pkg/move/check/target_state.go +++ b/pkg/move/check/target_state.go @@ -13,8 +13,15 @@ import ( "github.com/block/spirit/pkg/utils" ) +// TargetStateCheckName is the registered name of the target-state check — +// the one post-setup check that validates target-side state (tables absent, +// or empty with a matching schema) and therefore the only one that wiping +// the target can cure. The runner's --force path excludes it (via +// RunChecksExcluding) when deciding whether a wipe would actually help. +const TargetStateCheckName = "target_state" + func init() { - registerCheck("target_state", targetStateCheck, ScopePostSetup) + registerCheck(TargetStateCheckName, targetStateCheck, ScopePostSetup) } // targetStateCheck validates that target databases are ready for the move operation. diff --git a/pkg/move/runner.go b/pkg/move/runner.go index 4acb81a34..399188a14 100644 --- a/pkg/move/runner.go +++ b/pkg/move/runner.go @@ -46,6 +46,27 @@ var ( continuousChecksumMinInterval = 1 * time.Hour ) +// errCheckpointUnresumable marks checkpoint-resume failures that are +// definitive properties of the persisted checkpoint record itself: a +// positions payload that does not parse (e.g. a row written by the migration +// runner, which stores a single opaque position under the same +// _spirit_checkpoint table name) or a source missing from the positions map. +// Unlike transient failures (connectivity, locking) these can never succeed +// on retry, so --force may recover from them by wipe-and-restart. The other +// definitive state, a checkpoint past CheckpointMaxAge, keeps its existing +// sentinel status.ErrCheckpointTooOld. See isDefinitivelyUnresumable. +var errCheckpointUnresumable = errors.New("checkpoint is not resumable") + +// isDefinitivelyUnresumable reports whether a resumeFromCheckpoint error +// identifies a checkpoint that no retry can ever resume: it is too old +// (status.ErrCheckpointTooOld) or its persisted record is unusable +// (errCheckpointUnresumable). Only these states are covered by --force's +// wipe-and-restart contract; any other failure may be transient, and wiping +// on a transient error would destroy a target that could still resume. +func isDefinitivelyUnresumable(err error) bool { + return errors.Is(err, status.ErrCheckpointTooOld) || errors.Is(err, errCheckpointUnresumable) +} + // sourceInfo holds per-source connection state for N:M moves. type sourceInfo struct { db *sql.DB @@ -320,9 +341,80 @@ func (r *Runner) createTargetTables(ctx context.Context) error { } func (r *Runner) resumeFromCheckpoint(ctx context.Context) error { + // Read and validate the checkpoint record before this function causes any + // side effect (chunker subscriptions, runner state, target modifications). + // A definitive validation failure — the checkpoint is too old, or its + // positions payload is unusable — must leave the runner exactly as it + // found it: under --force, setup() reacts to it by falling through to the + // wipe-and-restart path, which rebuilds all of that state via newCopy(). + // + // Read checkpoint from targets[0] by convention. A checkpoint table written + // by an incompatible spirit version (e.g. missing or renamed a column) fails + // the read and aborts the move; we do not support cross-version resume. + rec, err := r.checkpointTbl().ReadLatest(ctx) + if err != nil { + return fmt.Errorf("could not read from checkpoint table '%s' on target: %w", checkpointTableName, err) + } + + // Check if the checkpoint is too old to safely resume — replaying many + // days of binary logs can be slower than re-copying, and the binlogs may + // have been purged anyway. Unlike migrate, move cannot silently fall back + // to a fresh copy: the target tables are non-empty (that is exactly why + // setup() chose the resume path), so we fail loudly and leave the decision + // to the operator — --force opts into the wipe-and-restart. + if checkpointAge := rec.Age(); checkpointAge >= r.move.CheckpointMaxAge { + return fmt.Errorf("%w: checkpoint is %s old (max allowed: %s). To proceed, either re-run with a larger --checkpoint-max-age, or re-run with --force to wipe the target tables (including '%s') and restart the move from scratch", + status.ErrCheckpointTooOld, + checkpointAge.Round(time.Second), + r.move.CheckpointMaxAge, + checkpointTableName, + ) + } + + // Parse per-source positions (opaque strings owned by the source impl), + // keyed by sourceKey (addr/dbname). A payload that does not parse — e.g. + // the row was written by the migration runner, which stores a single + // opaque position under the same _spirit_checkpoint table name — or that + // is missing a source can never be resumed by retrying, so both are + // tagged errCheckpointUnresumable for --force to act on. + var positions map[string]string + if err := json.Unmarshal([]byte(rec.Position), &positions); err != nil { + return fmt.Errorf("%w: could not parse binlog positions from checkpoint: %w", errCheckpointUnresumable, err) + } + for i := range r.sources { + if _, ok := positions[r.sources[i].sourceKey()]; !ok { + return fmt.Errorf("%w: checkpoint missing binlog position for source %s", errCheckpointUnresumable, r.sources[i].sourceKey()) + } + } + + copierWatermark := rec.CopierWatermark + r.checksumWatermark = rec.ChecksumWatermark + + // With multiple sources, a persisted checksum watermark cannot be trusted. + // deleteAboveWatermark (below) runs every (source, table) DELETE against + // every target, and same-named tables from different sources interleave in + // the target tables — so one source's DELETE also removes OTHER sources' + // rows below their own watermarks. Those rows are not recopied (each + // source's chunker resumes from its own watermark); only a checksum pass + // that runs from the very beginning detects and repairs the hole. Resuming + // the checksum at a watermark would skip re-verifying exactly the range + // where the hole sits, so discard it and force a full pass. + // + // With a single source the watermark is kept: deletes are per-table, and + // each table's delete range (above its watermark upper bound) is a subset + // of its recopy range (from the watermark lower bound), so nothing below + // the checksum watermark can have been deleted without being recopied. + if len(r.sources) > 1 && r.checksumWatermark != "" { + r.logger.Info("discarding persisted checksum watermark: multi-source resume requires a full checksum pass", + "reason", "deleteAboveWatermark may remove rows below other sources' watermarks; only a from-scratch checksum re-verifies and repairs them", + "sources", len(r.sources)) + r.checksumWatermark = "" + } + + // The checkpoint record is fully validated — everything from here on has + // side effects. copyChunkers := make([]table.Chunker, 0, len(r.sources)*len(r.sourceTables)) checksumChunkers := make([]table.Chunker, 0, len(r.sources)*len(r.sourceTables)) - var err error // For each source and each table, create a chunker and add a subscription // to that source's repl client. @@ -380,67 +472,6 @@ func (r *Runner) resumeFromCheckpoint(ctx context.Context) error { return err } - // Read checkpoint from targets[0] by convention. A checkpoint table written - // by an incompatible spirit version (e.g. missing or renamed a column) fails - // the read and aborts the move; we do not support cross-version resume. - tgt0 := &r.targets[0] - rec, err := r.checkpointTbl().ReadLatest(ctx) - if err != nil { - return fmt.Errorf("could not read from checkpoint table '%s' on target: %w", checkpointTableName, err) - } - copierWatermark := rec.CopierWatermark - r.checksumWatermark = rec.ChecksumWatermark - binlogPositionsJSON := rec.Position - - // Check if the checkpoint is too old to safely resume — replaying many - // days of binary logs can be slower than re-copying, and the binlogs may - // have been purged anyway. This must happen before any destructive step - // (deleteAboveWatermark below modifies the targets). Unlike migrate, - // move cannot silently fall back to a fresh copy: the target tables are - // non-empty (that is exactly why setup() chose the resume path), so we - // fail loudly and leave the decision to the operator. - if checkpointAge := rec.Age(); checkpointAge >= r.move.CheckpointMaxAge { - return fmt.Errorf("%w: checkpoint is %s old (max allowed: %s). To proceed, either re-run with a larger --checkpoint-max-age, or wipe the target tables (including '%s') and restart the move from scratch", - status.ErrCheckpointTooOld, - checkpointAge.Round(time.Second), - r.move.CheckpointMaxAge, - checkpointTableName, - ) - } - - // With multiple sources, a persisted checksum watermark cannot be trusted. - // deleteAboveWatermark (below) runs every (source, table) DELETE against - // every target, and same-named tables from different sources interleave in - // the target tables — so one source's DELETE also removes OTHER sources' - // rows below their own watermarks. Those rows are not recopied (each - // source's chunker resumes from its own watermark); only a checksum pass - // that runs from the very beginning detects and repairs the hole. Resuming - // the checksum at a watermark would skip re-verifying exactly the range - // where the hole sits, so discard it and force a full pass. - // - // With a single source the watermark is kept: deletes are per-table, and - // each table's delete range (above its watermark upper bound) is a subset - // of its recopy range (from the watermark lower bound), so nothing below - // the checksum watermark can have been deleted without being recopied. - if len(r.sources) > 1 && r.checksumWatermark != "" { - r.logger.Info("discarding persisted checksum watermark: multi-source resume requires a full checksum pass", - "reason", "deleteAboveWatermark may remove rows below other sources' watermarks; only a from-scratch checksum re-verifies and repairs them", - "sources", len(r.sources)) - r.checksumWatermark = "" - } - - // Parse per-source positions (opaque strings owned by the source impl), - // keyed by sourceKey (addr/dbname). - var positions map[string]string - if err := json.Unmarshal([]byte(binlogPositionsJSON), &positions); err != nil { - return fmt.Errorf("could not parse binlog positions from checkpoint: %w", err) - } - for i := range r.sources { - if _, ok := positions[r.sources[i].sourceKey()]; !ok { - return fmt.Errorf("checkpoint missing binlog position for source %s", r.sources[i].sourceKey()) - } - } - // Delete rows above the watermark from all target tables before resuming. // When resuming from a checkpoint, the keyAboveWatermark optimization // needs to know the highest key in the target table to avoid discarding @@ -466,6 +497,7 @@ func (r *Runner) resumeFromCheckpoint(ctx context.Context) error { } } + tgt0 := &r.targets[0] r.checkpointTable = table.NewTableInfo(tgt0.DB, tgt0.Config.DBName, checkpointTableName) r.usedResumeFromCheckpoint = true return nil @@ -560,26 +592,49 @@ func (r *Runner) setup(ctx context.Context) error { return probeErr // the probe itself failed transiently — don't wipe on a blip } if resumable { - if resumeErr := r.resumeFromCheckpoint(ctx); resumeErr != nil { + resumeErr := r.resumeFromCheckpoint(ctx) + if resumeErr == nil { + r.logger.Info("Successfully resumed move from existing checkpoint") + return nil + } + // resumeFromCheckpoint's deeper validations can still find the + // checkpoint definitively unusable (too old; positions that do not + // parse; a source missing from the positions map). Those are + // exactly the "cannot resume from a checkpoint" states --force + // promises to recover from, so fall through to the wipe path + // below. Anything else may be transient (connectivity, locking): + // hard-fail rather than wipe a target that could still resume on + // a retry. + if !r.move.Force || !isDefinitivelyUnresumable(resumeErr) { return fmt.Errorf("resume validation passed but checkpoint resume failed: %w", resumeErr) } - r.logger.Info("Successfully resumed move from existing checkpoint") - return nil + r.logger.Warn("force set and the checkpoint is definitively unresumable; falling back to a fresh copy", "reason", resumeErr) } if !r.move.Force { return fmt.Errorf("target state is invalid for both new copy and resume (re-run with --force to wipe the target and start fresh): %w", err) } + // Wiping the target only cures target-side state: the target_state + // check (non-empty tables, mismatched schema) and the checkpoint. The + // triggering failure may just as well have been source-side — + // rename_safety's leftover _old table, source_schema_consistency + // drift, table_compatibility — which RunChecks surfaces in arbitrary + // order (it iterates a map). Re-run every post-setup check except + // target_state BEFORE wiping, so a move that would still fail + // afterwards fails now, with the target (possibly a large partial + // copy plus a valid checkpoint) intact. + if preErr := r.runChecksExcluding(ctx, check.ScopePostSetup, check.TargetStateCheckName); preErr != nil { + return fmt.Errorf("force: refusing to wipe the target because a check that wiping cannot fix is failing: %w", preErr) + } r.logger.Warn("force set and the target cannot resume; wiping target tables and starting fresh") if werr := r.wipeTargets(ctx); werr != nil { return fmt.Errorf("force: failed to wipe target before a fresh copy: %w", werr) } // --force only overrides the target-not-empty decision — it must not // bypass the other post-setup safety checks (rename_safety, - // source_schema_consistency, ...). Re-run them against the now-wiped - // target (target_state passes for the absent tables, exactly as a fresh - // move); abort if anything still fails rather than copying into a state - // that would only fail at cutover. RunChecks iterates a map, so the - // original failure was not necessarily target_state. + // source_schema_consistency, ...). Re-run the full set against the + // now-wiped target (target_state passes for the absent tables, exactly + // as a fresh move); abort if anything still fails rather than copying + // into a state that would only fail at cutover. if err := r.runChecks(ctx, check.ScopePostSetup); err != nil { return fmt.Errorf("force: target still fails post-setup checks after wiping: %w", err) } @@ -1057,8 +1112,9 @@ func (r *Runner) SetLogger(logger *slog.Logger) { r.logger = logger } -// runChecks wraps around check.RunChecks and adds the context of this move operation -func (r *Runner) runChecks(ctx context.Context, scope check.ScopeFlag) error { +// checkResources assembles the check.Resources describing this move +// operation, shared by runChecks and runChecksExcluding. +func (r *Runner) checkResources() check.Resources { sources := make([]check.SourceResource, len(r.sources)) for i := range r.sources { sources[i] = check.SourceResource{ @@ -1067,14 +1123,26 @@ func (r *Runner) runChecks(ctx context.Context, scope check.ScopeFlag) error { DSN: r.sources[i].dsn, } } - return check.RunChecks(ctx, check.Resources{ + return check.Resources{ Sources: sources, Targets: r.targets, SourceTables: r.sourceTables, CreateSentinel: r.move.CreateSentinel, GTID: r.move.EnableExperimentalGTID, MoveEverything: len(r.move.SourceTables) == 0, - }, r.logger, scope) + } +} + +// runChecks wraps around check.RunChecks and adds the context of this move operation +func (r *Runner) runChecks(ctx context.Context, scope check.ScopeFlag) error { + return check.RunChecks(ctx, r.checkResources(), r.logger, scope) +} + +// runChecksExcluding is runChecks minus the named checks. The --force path +// uses it to ask "would the post-setup checks still fail for a reason that +// wiping the target cannot cure?" before destroying any target data. +func (r *Runner) runChecksExcluding(ctx context.Context, scope check.ScopeFlag, exclude ...string) error { + return check.RunChecksExcluding(ctx, r.checkResources(), r.logger, scope, exclude...) } // restoreSecondaryIndexes restores any secondary indexes that were deferred during table creation. diff --git a/pkg/move/runner_test.go b/pkg/move/runner_test.go index 134584683..2db932467 100644 --- a/pkg/move/runner_test.go +++ b/pkg/move/runner_test.go @@ -511,6 +511,77 @@ func TestMoveForceWipesUnresumableTarget(t *testing.T) { require.Zero(t, stale, "force must drop+recreate the target, not overlay the source onto stale rows") } +// TestMoveForcePreservesTargetOnSourceSideFailure verifies that the --force +// wipe is gated on the failure actually being curable by a wipe. Wiping the +// target only cures target-side state (the target_state check and the +// checkpoint); a source-side failure — here rename_safety's leftover t1_old +// on the source — would make the move fail identically after the wipe, +// destroying a possibly large partial copy for nothing. --force must fail +// WITHOUT touching the target, and only wipe once the source-side problem is +// gone. Before the fix, the wipe ran first and the checks after, so the +// partial copy and checkpoint were destroyed and the run still failed. +func TestMoveForcePreservesTargetOnSourceSideFailure(t *testing.T) { + srcDB := "source_force_srcside" + dstDB := "dest_force_srcside" + sourceDSN := testutils.DSNForDatabase(srcDB) + targetDSN := testutils.DSNForDatabase(dstDB) + + testutils.RunSQL(t, "DROP DATABASE IF EXISTS "+srcDB) + testutils.RunSQL(t, "DROP DATABASE IF EXISTS "+dstDB) + testutils.RunSQL(t, "CREATE DATABASE "+srcDB) + testutils.RunSQL(t, "CREATE DATABASE "+dstDB) + testutils.RunSQL(t, "CREATE TABLE "+srcDB+".t1 (id INT NOT NULL PRIMARY KEY AUTO_INCREMENT, name VARCHAR(50) NOT NULL)") + testutils.RunSQL(t, "INSERT INTO "+srcDB+".t1 (name) VALUES ('a'),('b'),('c'),('d'),('e')") + // The source-side problem: a leftover t1_old from a previous move. The + // cutover renames t1 to t1_old, so rename_safety fails while it exists — + // and no amount of wiping the TARGET can fix it. + testutils.RunSQL(t, "CREATE TABLE "+srcDB+".t1_old (id INT NOT NULL PRIMARY KEY)") + + // Target: a partial copy from a prior run (the id-999 row marks it) plus a + // checkpoint from an incompatible spirit version, so resume is impossible — + // the same unresumable-target shape as TestMoveForceWipesUnresumableTarget. + testutils.RunSQL(t, "CREATE TABLE "+dstDB+".t1 (id INT NOT NULL PRIMARY KEY AUTO_INCREMENT, name VARCHAR(50) NOT NULL)") + testutils.RunSQL(t, "INSERT INTO "+dstDB+".t1 (id, name) VALUES (999, 'precious')") + testutils.RunSQL(t, "CREATE TABLE "+dstDB+"._spirit_checkpoint (id INT NOT NULL AUTO_INCREMENT PRIMARY KEY, copier_watermark TEXT, checksum_watermark TEXT, binlog_positions TEXT, statement TEXT, created_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP)") + testutils.RunSQL(t, "INSERT INTO "+dstDB+"._spirit_checkpoint (copier_watermark, binlog_positions) VALUES ('stale-wm', '{}')") + + newMove := func() *Move { + return &Move{ + SourceDSN: sourceDSN, + TargetDSN: targetDSN, + TargetChunkTime: 100 * time.Millisecond, + Threads: 2, + WriteThreads: 2, + Force: true, + } + } + + // --force with the source-side failure: the run must fail on rename_safety + // and must NOT have wiped the target. + err := newMove().Run() + require.Error(t, err) + require.ErrorContains(t, err, "refusing to wipe") + require.ErrorContains(t, err, "t1_old") + + targetDB, err := sql.Open("mysql", targetDSN) + require.NoError(t, err) + defer utils.CloseAndLog(targetDB) + var count int + require.NoError(t, targetDB.QueryRowContext(t.Context(), "SELECT COUNT(*) FROM t1 WHERE id = 999").Scan(&count)) + require.Equal(t, 1, count, "a source-side failure must not wipe the partial copy on the target") + require.NoError(t, targetDB.QueryRowContext(t.Context(), "SELECT COUNT(*) FROM "+checkpointTableName).Scan(&count)) + require.Equal(t, 1, count, "a source-side failure must not drop the checkpoint on the target") + + // Clear the source-side problem: --force can now cure what is left (the + // unresumable target) by wiping, and the move completes. + testutils.RunSQL(t, "DROP TABLE "+srcDB+".t1_old") + require.NoError(t, newMove().Run()) + require.NoError(t, targetDB.QueryRowContext(t.Context(), "SELECT COUNT(*) FROM t1").Scan(&count)) + require.Equal(t, 5, count, "after the source-side failure is fixed, force must wipe and re-copy") + require.NoError(t, targetDB.QueryRowContext(t.Context(), "SELECT COUNT(*) FROM t1 WHERE id = 999").Scan(&count)) + require.Zero(t, count, "the stale partial-copy row must be gone after the wipe") +} + // TestMoveWithVarcharPK verifies a move on a table with a non-memory-comparable // primary key (VARCHAR with a CI collation) — the case from issue #607. The // move runs under concurrent writes to exercise the binlog replay path. @@ -1264,6 +1335,123 @@ func TestResumeFromCheckpointNotTooOld(t *testing.T) { require.NoError(t, r.Close()) } +// testForceRecoversUnresumableCheckpoint is the shared body for the +// --force-recovers-a-definitively-unresumable-checkpoint tests. It produces a +// real checkpoint on the target via checkpointAndStop, lets the caller corrupt +// the checkpoint row into a state that passes the resume probe but fails +// resumeFromCheckpoint's deeper validation, then asserts the contract from +// both sides: +// - without --force the move hard-fails with the typed sentinel, leaving the +// target's rows and checkpoint untouched; +// - with --force the move wipes the target and completes as a fresh copy +// (usedResumeFromCheckpoint stays false) instead of hard-failing. +func testForceRecoversUnresumableCheckpoint(t *testing.T, suffix string, corrupt func(dstDB string), wantSentinel error, wantErrContains string) { + srcDB := "source_force_" + suffix + dstDB := "dest_force_" + suffix + sourceDSN := testutils.DSNForDatabase(srcDB) + targetDSN := testutils.DSNForDatabase(dstDB) + + testutils.RunSQL(t, "DROP DATABASE IF EXISTS "+srcDB) + testutils.RunSQL(t, "DROP DATABASE IF EXISTS "+dstDB) + testutils.RunSQL(t, "CREATE DATABASE "+srcDB) + testutils.RunSQL(t, "CREATE DATABASE "+dstDB) + + // Same seeding as TestResumeFromCheckpointTooOld: enough rows for several + // chunks so the copier watermark is ready when checkpointAndStop dumps the + // checkpoint (1 -> 2 -> 10 -> 1010 rows). + testutils.RunSQL(t, "CREATE TABLE "+srcDB+".t1 (id INT NOT NULL PRIMARY KEY AUTO_INCREMENT, val VARBINARY(64))") + testutils.RunSQL(t, "INSERT INTO "+srcDB+".t1 (val) SELECT RANDOM_BYTES(64)") + for range 3 { + testutils.RunSQL(t, "INSERT INTO "+srcDB+".t1 (val) SELECT RANDOM_BYTES(64) FROM "+srcDB+".t1 a JOIN "+srcDB+".t1 b JOIN "+srcDB+".t1 c LIMIT 5000") + } + + move := &Move{ + SourceDSN: sourceDSN, + TargetDSN: targetDSN, + TargetChunkTime: 100 * time.Millisecond, + Threads: 1, + WriteThreads: 1, + } + checkpointAndStop(t, move) + corrupt(dstDB) + + sourceDB, err := sql.Open("mysql", sourceDSN) + require.NoError(t, err) + defer utils.CloseAndLog(sourceDB) + targetDB, err := sql.Open("mysql", targetDSN) + require.NoError(t, err) + defer utils.CloseAndLog(targetDB) + // Capture the expected row count now: the successful --force run below + // ends with a cutover that renames the source t1 away. + var srcCount int + require.NoError(t, sourceDB.QueryRowContext(t.Context(), "SELECT COUNT(*) FROM t1").Scan(&srcCount)) + + // Without --force: the resume probe passes (the checkpoint row reads fine + // with move's schema) but the deeper validation fails; this must stay a + // hard error and must not touch the target. + r, err := NewRunner(move) + require.NoError(t, err) + err = r.Run(t.Context()) + require.Error(t, err) + require.ErrorIs(t, err, wantSentinel) + require.ErrorContains(t, err, wantErrContains) + require.False(t, r.usedResumeFromCheckpoint) + require.NoError(t, r.Close()) + + var count int + require.NoError(t, targetDB.QueryRowContext(t.Context(), "SELECT COUNT(*) FROM t1").Scan(&count)) + require.Equal(t, srcCount, count, "without --force the target's copied rows must be left untouched") + require.NoError(t, targetDB.QueryRowContext(t.Context(), "SELECT COUNT(*) FROM "+checkpointTableName).Scan(&count)) + require.Equal(t, 1, count, "without --force the checkpoint must be left untouched") + + // With --force: the definitive unresumable state falls through to + // wipe-and-restart. Before the fix this hard-failed with "resume + // validation passed but checkpoint resume failed" even with --force set, + // leaving the operator to DROP the target tables by hand. + move.Force = true + r, err = NewRunner(move) + require.NoError(t, err) + require.NoError(t, r.Run(t.Context())) + require.False(t, r.usedResumeFromCheckpoint, "--force must wipe and start fresh, not resume") + require.NoError(t, r.Close()) + require.NoError(t, targetDB.QueryRowContext(t.Context(), "SELECT COUNT(*) FROM t1").Scan(&count)) + require.Equal(t, srcCount, count, "the fresh copy must move all source rows") +} + +// TestMoveForceRecoversUnparseableCheckpointPositions covers the checkpoint +// shape a migration-runner row leaves in the same _spirit_checkpoint table +// name: binlog_position holds a single opaque position, not move's JSON map of +// per-source positions. The resume probe passes but the positions can never +// parse, so the state is definitively unresumable: a hard error without +// --force, wipe-and-restart with it. +func TestMoveForceRecoversUnparseableCheckpointPositions(t *testing.T) { + testForceRecoversUnresumableCheckpoint(t, "badpos", func(dstDB string) { + testutils.RunSQL(t, "UPDATE "+dstDB+"."+checkpointTableName+" SET binlog_position = 'mysql-bin.000123:4567'") + }, errCheckpointUnresumable, "could not parse binlog positions") +} + +// TestMoveForceRecoversCheckpointMissingSource covers a checkpoint whose +// positions payload parses but does not contain this move's source key (e.g. +// the checkpoint belongs to a move of a different source into the same +// target). Definitively unresumable: a hard error without --force, +// wipe-and-restart with it. +func TestMoveForceRecoversCheckpointMissingSource(t *testing.T) { + testForceRecoversUnresumableCheckpoint(t, "nosrckey", func(dstDB string) { + testutils.RunSQL(t, "UPDATE "+dstDB+"."+checkpointTableName+" SET binlog_position = '{}'") + }, errCheckpointUnresumable, "checkpoint missing binlog position for source") +} + +// TestMoveForceRecoversTooOldCheckpoint complements +// TestResumeFromCheckpointTooOld (which pins the hard failure without +// --force): a checkpoint past CheckpointMaxAge is a definitive unresumable +// state, so with --force the move must wipe the target and restart fresh +// instead of failing. +func TestMoveForceRecoversTooOldCheckpoint(t *testing.T) { + testForceRecoversUnresumableCheckpoint(t, "oldchkpt", func(dstDB string) { + testutils.RunSQL(t, "UPDATE "+dstDB+"."+checkpointTableName+" SET created_at = DATE_SUB(NOW(), INTERVAL 8 DAY)") + }, status.ErrCheckpointTooOld, "re-run with a larger --checkpoint-max-age") +} + // TestCreateSentinelTableIdempotent verifies that sentinel.Create // adopts an existing sentinel rather than DROP+CREATE-ing it. The sentinel // name is shared with concurrent spirit processes polling it every