Skip to content

feat(engine): disclose at plan time whether an apply continues or discards a copy - #1087

Draft
aparajon wants to merge 5 commits into
mainfrom
armand/copy-plan-disclosure
Draft

feat(engine): disclose at plan time whether an apply continues or discards a copy#1087
aparajon wants to merge 5 commits into
mainfrom
armand/copy-plan-disclosure

Conversation

@aparajon

@aparajon aparajon commented Aug 19, 2026

Copy link
Copy Markdown
Collaborator

Why this matters

The engine can already predict whether an apply will resume unfinished work on the target or destroy it, but only at apply time — the moment the decision is unavoidable. By then the copy is gone. The point of predicting it at all is to tell someone before they confirm, and that requires the plan to carry the answer.

What it does

Runs the same prediction at plan time, against the target the plan already reads, and carries the result on the plan as an engine-agnostic ExistingCopy per namespace.

plan
  └─ for each namespace the plan touches
       └─ does a copy exist for a table in this batch?
            ├─ no  → PlanResult carries nothing
            └─ yes → PlanResult.ExistingCopies += { namespace, disposition, reason, tables, age }

The shape is one entry per namespace, not one per plan, so an engine that spans several targets reports which one each disclosure is about. A single-database engine produces at most one and a caller planning several concatenates them.

Two safety properties worth calling out:

  • A plan is a read. It describes the target and must never fail because of it. A target that cannot be read is logged and the plan carries no disclosure, leaving the plan exactly as it is without the check.
  • A plan-time miss is not the last word. The same prediction runs again at apply time against the routing the apply actually took, so a target that was unreachable while planning is still evaluated before anything is destroyed.

The type carries no engine-specific fields — namespace, disposition, reason, tables, age — so a second engine that copies tables reports the same shape and every surface renders it one way.

How it moves us toward the northstar

This is the middle link of three. The engineer whose schema change is about to destroy a day of copying learns nothing from an engine's internal state — they read a plan, in a PR comment or on the CLI. The plan is the one object every surface already reads, so putting the disposition there is what lets any of them disclose it without reaching into an engine. #1048 makes the prediction; #1088 renders it.

Opened by Claude (Opus 5).

…ll do to it

An apply that meets a partly finished row copy on the target either
continues it or throws it away and starts over. Today the only trace is
one Spirit info line naming neither the tables nor the copy's age, so a
discard that destroys days of copying passes unnoticed.

Find the copy before Spirit runs — its shadow tables and the checkpoint
recording the batch they were made for — and work out the disposition
the same way Spirit will, comparing the joined batch against the
recorded statement. Log it with the copied tables, the checkpoint read,
and the copy's age: a warning for a discard, with its reason, and info
for an adopt.
@aparajon
aparajon force-pushed the armand/adopt-discard-disclosure branch from a701b89 to 9bf565c Compare August 19, 2026 08:42
@aparajon
aparajon force-pushed the armand/copy-plan-disclosure branch from 70b8935 to ad63e51 Compare August 19, 2026 08:42
Three corrections to the adopt prediction, all in the direction that
matters: an optimistic adopt promises survival to work that is then
destroyed.

A batch that covers more tables than the copy does was predicted adopt.
Spirit reads the shadow table of every table in the batch before it
resumes and rebuilds all of them if any one is missing, so the tables
that were copied are dropped along with the ones that never started --
even when the checkpoint is current and its statement matches byte for
byte. Adopt now requires the copy to cover the whole batch, reported as
a distinct copy_incomplete reason because the operator did nothing to
cause it and cannot avoid it by restoring the batch.

The age bound is now inclusive, matching Spirit's own comparison. This
prediction measures the age strictly earlier than Spirit does, so a
checkpoint exactly at the limit can only be older by the time Spirit
looks.

The lookup now carries its own deadline. Neither caller's context has
one: a plan serves an interactive comment and an apply runs for hours.
A target that accepted a connection and then stopped answering could
stall the dial or either query for as long as the operating system
allowed, which is a describing read holding up the apply it describes.

The discard log names the batch alongside the copied tables, so the
incomplete case is readable without a second lookup.
@aparajon
aparajon force-pushed the armand/copy-plan-disclosure branch from ad63e51 to fc819c4 Compare August 20, 2026 06:24
An operator who stops a copy and starts it again days later meets the
discard that costs the most: the checkpoint has expired, so Spirit rebuilds
from zero on the very start the operator asked for. That apply reached
Spirit through the resume path, which hands over the stored statement
without going back through routing, so it never passed the report and said
nothing about the copy it destroyed.

The disclosure moves to the point both paths share, where the string handed
to Spirit is already final, so a first apply and a resumed one report the
same thing and no later path can miss it.

An unreachable checkpoint no longer reports an age. Zero read as "written
moments ago", which understates a copy that may be days old.
@aparajon
aparajon force-pushed the armand/copy-plan-disclosure branch from fc819c4 to a050715 Compare August 20, 2026 07:46
Copy progress says chunks are landing, not that the copy can be resumed:
the checkpoint records the contiguous low watermark, and the copier feeds
chunks back out of order as their writes finish. Halting on rows copied
alone raced the watermark, so the helper occasionally left a shadow table
with no checkpoint behind it and the resume tests failed on a copy that was
never resumable in the first place.

Waiting for the watermark first is what the shutdown tests already do
before asserting on a halt's checkpoint.
…cards a copy

The prediction already runs at apply time, once the decision is
unavoidable. Run it at plan time too, against the target the plan reads,
and carry the answer on the plan as an engine-agnostic ExistingCopy per
namespace.

A plan is a read: it describes the target and must never fail because of
it, so a target that cannot be read is logged and the plan carries no
disclosure, leaving the plan exactly as it is without the check.
@aparajon
aparajon force-pushed the armand/copy-plan-disclosure branch from a050715 to 645389f Compare August 20, 2026 07:56
Base automatically changed from armand/adopt-discard-disclosure to main August 20, 2026 08:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant