copier: remove the legacy unbuffered copier

[morgo]

Why

The buffered DBLog-style copier has been the default since v0.15.0 (#908), and move/sync have always required it. A search across Block automation and downstream consumers (tern, strata) found nothing setting --unbuffered, so this retires the legacy INSERT IGNORE .. SELECT implementation rather than keep maintaining two copy paths. Net −832 lines.

What changed

Code

• Delete pkg/copier/unbuffered.go and the --unbuffered flag.
• CopierConfig loses Unbuffered and the vestigial TargetChunkTime (chunk sizing lives entirely in the chunker); NewCopier drops its unused *sql.DB parameter and now requires a non-nil Applier.
• The migration runner's copy chunkers always size by TargetChunkBytes now; --target-chunk-time remains for the checksum. Also fixes the startup log printing the TargetChunkTime value under the target-chunk-size key.
• The autoscaling-vs-unbuffered downgrade warning is gone (the combination can no longer exist).
• Runner.Close() now stops the applier. Run's teardown already stopped it (Stop is idempotent), but early-failure paths — and tests that step the copy incrementally — would otherwise leak the applier's worker goroutines.

New API: copier.ChunkCopier

The unbuffered copier's synchronous CopyChunk was the vehicle for every deterministic-stepping test (checkpoint/watermark ordering, binlog interleaving). The buffered copier now implements the same contract behind a small interface:

type ChunkCopier interface {
    CopyChunk(ctx context.Context, chunk *table.Chunk) error
}

It auto-starts the applier (idempotent), applies the chunk, and waits on the applier callback — feedback reaches the chunker before it returns, so tests can complete chunks in a controlled order (e.g. 2, 1, 3) exactly as before.

Displaced test coverage

• Range-optimizer refusal (range_optimizer_max_mem_size → fatal) moved to pkg/dbconn, where the warning-code classification actually lives.
• The NULL→NOT NULL "unsafe warning" assertion is ported to the buffered path.
• Lossy-conversion behavior stays covered by TestBufferedCopierDataTypeConversionError.
• TestResumeFromCheckpointPhantom is deleted, not ported: per its own doc comment the phantom row only arises on the INSERT IGNORE ... SELECT recopy path; the buffered copier applies row images via REPLACE and has no equivalent scenario.
• All buffered/unbuffered test matrices collapse to single runs (rename-column, datatype, cutover-atomicity incl. the semisync build-tag variant, migration_test pairs).

Docs

• docs/migrate.md drops the ### unbuffered section; README/AGENTS/package READMEs (copier, applier, table, throttler, migration) now describe the single-copier world, keeping short history notes where the contrast is still instructive.

Verification

• go build ./..., go vet (including semisync and singleversion build tags), gofmt, golangci-lint (0 issues, uncapped).
• Against local MySQL 8.0.43: full pkg/copier (goleak-guarded), pkg/change, pkg/move, pkg/datasync; pkg/dbconn -run TestRangeOptimizerRefusal; focused pkg/migration runs covering the ported step-through tests (TestCheckpoint, TestCheckpointRestore, all 7 TestE2EBinlogSubscribing*), resume E2E, and every collapsed matrix (16 rename tests, enum/set reorder+drop, cutover atomicity, generated-columns/binary-checksum/charset pairs). All pass.

🤖 Generated with Claude Code

[Copilot]

Pull request overview

This PR removes Spirit’s legacy unbuffered (INSERT IGNORE ... SELECT) copier and its CLI/config surface area, consolidating the system on the DBLog-style buffered copier across migrate, move, and sync/datasync. It also introduces a small incremental copy interface (copier.ChunkCopier) to preserve deterministic “step chunk-by-chunk” testing without keeping the unbuffered implementation.

Changes:

• Deleted the unbuffered copier implementation and removed the --unbuffered flag/config plumbing.
• Simplified copier construction and config (NewCopier signature change; CopierConfig trims legacy fields) and added ChunkCopier.CopyChunk for deterministic stepping tests.
• Adjusted runners and docs to reflect byte-budget chunk sizing for copy, time-budget chunk sizing for checksum; tightened teardown by stopping the applier in Runner.Close().

Reviewed changes

Copilot reviewed 34 out of 34 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
README.md	Updates dynamic chunking explanation for single-copier world.
AGENTS.md	Updates architecture notes to reflect buffered-only copier.
docs/README.md	Updates docs overview to remove unbuffered mode mention.
docs/migrate.md	Removes --unbuffered docs; clarifies chunk sizing responsibilities.
pkg/applier/README.md	Updates applier role/relationship now that copier is buffered-only.
pkg/change/binlog_test.go	Updates copier construction API usage in tests.
pkg/checksum/json_test.go	Updates historical wording re: legacy server-side copy.
pkg/copier/README.md	Rewrites copier docs around buffered-only algorithm + new ChunkCopier.
pkg/copier/copier.go	Removes unbuffered selection; adds ChunkCopier interface; updates NewCopier signature and validation.
pkg/copier/buffered.go	Adds synchronous CopyChunk implementation to support deterministic tests.
pkg/copier/unbuffered.go	Deletes legacy unbuffered copier implementation.
pkg/copier/copier_test.go	Collapses tests onto buffered path; updates NewCopier API usage.
pkg/copier/buffered_test.go	Updates construction and chunker config usage after config/API changes.
pkg/copier/autoscaler_test.go	Updates construction API usage.
pkg/dbconn/dbconn_test.go	Moves range-optimizer refusal coverage into dbconn tests.
pkg/datasync/runner.go	Updates copier construction API usage.
pkg/move/runner.go	Updates copier construction API usage.
pkg/migration/migration.go	Removes Unbuffered flag/field; clarifies chunk sizing docs for copy vs checksum.
pkg/migration/runner.go	Updates copier construction; fixes startup logging; always sizes copy chunks by bytes; stops applier in Close().
pkg/migration/README.md	Updates locking explanation now that unbuffered is removed.
pkg/migration/check/rename.go	Updates rename-check comment to remove dual-mode wording.
pkg/migration/helpers_test.go	Removes WithBuffered helper option.
pkg/migration/resume_test.go	Updates deterministic stepping tests to use copier.ChunkCopier; removes unbuffered-only phantom test.
pkg/migration/binlog_test.go	Updates deterministic stepping tests to use copier.ChunkCopier.
pkg/migration/rename_column_test.go	Collapses buffered/unbuffered matrices into single-mode tests.
pkg/migration/migration_test.go	Collapses buffered/unbuffered matrices into single-mode tests.
pkg/migration/datatype_test.go	Collapses buffered/unbuffered matrices into single-mode tests.
pkg/migration/cutover_test.go	Collapses buffered/unbuffered matrices into single-mode tests.
pkg/migration/cutover_semisync_test.go	Collapses buffered/unbuffered matrices into single-mode tests.
pkg/table/README.md	Updates chunker docs to remove unbuffered distinctions.
pkg/table/chunk.go	Updates comments re: who sets ActualBytes.
pkg/table/chunker.go	Updates comments re: time vs byte signals.
pkg/table/dynamic_chunk_sizer.go	Updates comments re: time-signal path participants.
pkg/throttler/README.md	Updates example usage to new copier API (needs applier wiring to be complete).

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.