Skip to content

fix(tern): settle sequential tasks when the engine loses in-flight work - #1113

Open
aparajon wants to merge 4 commits into
mainfrom
armand/poll-lost-work
Open

fix(tern): settle sequential tasks when the engine loses in-flight work#1113
aparajon wants to merge 4 commits into
mainfrom
armand/poll-lost-work

Conversation

@aparajon

@aparajon aparajon commented Aug 23, 2026

Copy link
Copy Markdown
Collaborator

What breaks today

The loop driving one table's schema change polls the engine until it reports the work finished or failed. That is the loop's only exit — so if the engine forgets the work, there is no exit.

Engines do forget. Their view of in-flight work lives in one process's memory, and it vanishes when that process restarts, or when a sibling drive on the same pod drains the engine in the window between the work finishing and the next poll. From then on the engine answers "no active schema change", with no error, forever — which reads exactly like a stale poll, and the loop is built to ride stale polls out.

                            engine forgets the work it accepted
                                            ╳
   poll ─────── poll ─────── poll ─────── poll ─────── poll ─────── …
  running      running      running     "nothing"    "nothing"
                                                          │
                              stored state still says "in flight",
                              so the drive waits it out. Forever.

Nothing notices, because from the outside nothing is wrong: heartbeats stay healthy, the error budget never burns, recovery never fires. Meanwhile the apply holds the database's one active-apply slot, so every later apply to that database queues behind work that will never finish.

The fix

When the engine reports no active work but stored state says the work is in flight, ask the target database — the one authority that cannot forget an outcome.

  what the target shows        the task settles as
  ──────────────────────────────────────────────────────────
  mid-revert (never read)  →   retryable
  change is present        →   completed
  change is missing        →   retryable, a fresh claim re-drives it

The catch is that "no active work" is overloaded. An engine that provisions after accepting work — cutting a branch, validating a deploy request — says the same thing about real, healthy work that simply hasn't started yet, and bouncing one of those re-drives an apply that was about to run fine. So engines declare how to read their own answer, through a new optional engine.SynchronousWorkRegistration. Spirit declares it: it runs in this process with nothing to provision, so once Apply has returned the work is either running or gone and the first such report is conclusive. Undeclared engines are assumed to provision and get a bounded wait first, which is the safe default.

That wait is a duration rather than a count of polls, because what it has to outlast is engine behaviour measured in wall-clock time; a poll count would silently shrink to nothing if someone shortened the poll interval.

The revert case is the sharp edge. After a forward change cuts over, the live schema matches the reviewed target by definition — so a match says nothing about whether the revert this task was driving ever finished, and completing on it would report a successful schema change while the revert is gone. A revert-phase task settles retryable without reading the target at all, the same guard the resume path already applies. Beyond that, verification can only ever land failed_retryable, never failed, and errors reading the target burn the existing consecutive-error budget so this path can't become a second silent loop.

Also: the drive says when it's stuck

The same loop gains a stall watchdog. When a task's state and progress counters haven't moved for the warning interval, it logs once per interval with the task's triage attributes, how long it's been motionless, and what the engine reports. It observes only — it never changes task state. Tasks parked at an operator gate (held cutover, deferred deploy, open revert window) are motionless by design and stay quiet, and since the watchdog still sees every poll, entering or leaving a gate restarts its clock.


Engine progress is a display feed; outcomes belong to durable state. This is the first drive path where the target database settles a task when the engine's in-memory view and stored state disagree — progress polls decide what operators see, durable state and the target decide what is true. The capability interface follows the precedent ExternallyAuthoritativeProgress set: instance-local memory is never trusted as truth unless an engine explicitly says it can be.

PR summary written by Claude Code (Opus 5).

Copilot AI lite review requested due to automatic review settings August 23, 2026 04:24

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens the sequential Tern drive against “lost in-flight work” scenarios where the engine reports no active schema change while storage still shows a task running, which previously could lead to silent infinite polling and a wedged apply.

Changes:

  • Adds bounded “lost work” detection in the sequential poll loop and verifies target convergence via a re-plan when the pending/no-active signal persists.
  • Introduces a stall watchdog that rate-limits warnings when task state/progress does not move for a configured interval.
  • Expands sequential progress tests to cover lost-work settlement (converged vs not), stale-snapshot self-heal, and watchdog warning behavior.

Reviewed changes

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

File Description
pkg/tern/local_client.go Adds LocalClient overrides for sequential poll cadence and stall-warning interval (primarily for tests).
pkg/tern/local_apply_sequential.go Implements lost-work tolerance + verification, bounded error handling integration, and a stall watchdog with rate-limited warnings.
pkg/tern/local_apply_sequential_progress_test.go Adds fixtures and tests for lost-work settlement paths and watchdog logging/rate limiting.

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

Comment thread pkg/tern/local_apply_sequential.go
Comment thread pkg/tern/local_apply_sequential.go
The sequential drive's progress poll tolerates a short window of the
engine reporting no active schema change for an in-flight task (a stale
snapshot after an engine restart self-heals), then stops trusting the
engine and verifies the target schema directly: a converged target
completes the task through the normal completed flow, a target that
still needs the change marks the task retryable so a fresh claim
re-drives it, and verification errors count against the poll's bounded
consecutive-error budget. Every branch logs with the task's triage
attributes and the engine-reported state.

The poll also carries a stall watchdog: a task sitting in the same
state with unchanged progress fields for a full interval is warned
about once per interval, without changing task state.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@aparajon
aparajon force-pushed the armand/poll-lost-work branch from e950fc7 to 9f7610a Compare August 24, 2026 03:57
@aparajon
aparajon marked this pull request as ready for review August 24, 2026 04:27
aparajon and others added 3 commits August 24, 2026 12:50
…nd trust the engine on a clock

A task in its revert phase is in flight, so the sequential drive's lost-work
verification could reach it. Reading the target schema cannot settle a revert:
the forward change has already cut over, so the live schema matches the
reviewed target by definition and a match says nothing about whether the revert
ever finished. That path reported the apply as a successful schema change while
the revert it was undoing was gone. A revert-phase task is now marked retryable
without consulting the target, matching the guard the resume path already
applies for the same reason.

The trust budget before that verification runs is now a duration rather than a
count of polls. What it has to outlast is wall-clock engine behaviour, not a
number of round trips: an engine that just restarted serves a stale snapshot
until it catches up, and an engine whose remote work is still being provisioned
or validated reports no active schema change for real, healthy work it has not
begun executing. A poll-count budget silently shrinks to nothing whenever the
poll cadence is shortened, which bounced a healthy apply to retryable and
re-drove it — worse than waiting on an engine that provisions remote resources
per attempt.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… an operator gate

A held cutover, a deferred deploy and an open revert window are all states the
drive is meant to sit in without moving, because the next step belongs to an
operator rather than to the engine. Progress fields do not advance there by
design, so the stall warning fired once per interval for as long as the
operator took to act — on every deferred cutover and deferred deploy. The
watchdog still observes every poll, so entering or leaving one of these states
restarts its clock and a genuinely stuck task still warns.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The trust budget before the drive verifies the target schema was one constant
for every engine, sized for the slowest of them. That is the wrong shape: how
long a pending progress report stays ambiguous is a property of the engine, not
of the drive.

An engine that provisions after accepting work — cutting a branch, opening and
validating a deploy request — reports pending for real, healthy work for as
long as that setup takes, so a driver must give it time. An engine that
publishes its tracked schema change before Apply returns has no such phase:
once Apply has returned the work is either running or it is gone, so the first
pending report about an in-flight task is already conclusive and there is
nothing to wait out.

A new optional engine.SynchronousWorkRegistration lets an engine declare which
it is, and the sequential drive reads that declaration to size the budget.
Spirit declares it: it runs in a goroutine of this process with nothing to
provision, and Drain and Cancel are the only writers that clear the tracked
state, both of which mean the work is not coming back. Engines that do not
declare it keep the full budget, which is the safe default — provisioning is
never mistaken for lost work.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.

2 participants