Skip to content

fix(resync): publish off the reconcile pass's thread - #437

Merged
schickling merged 2 commits into
mainfrom
schickling-assistant/2026-09-04-2026-09-04-resync-refusal-lock-starvation
Sep 4, 2026
Merged

fix(resync): publish off the reconcile pass's thread#437
schickling merged 2 commits into
mainfrom
schickling-assistant/2026-09-04-2026-09-04-resync-refusal-lock-starvation

Conversation

@schickling-assistant

@schickling-assistant schickling-assistant commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Problem

A reconcile pass could not complete while resync publication was making no progress.

ResyncSupervisor::install_live and deactivate block on an acknowledgement from the resync
worker thread, once per live seat per pass. That same thread ran every publication, and a
publication is neither cheap nor bounded: emit_admitted takes the shared catalog-authoring lock,
re-resolves the whole catalog through discover_strict, and takes the recipient's stream lock. So
a publication that was slow, refused-and-retried, or blocked on another process serialized the
whole pass behind it.

That is the property which turned a noisy retry into a two-hour outage in #431. A resync refused
because its recipient is suspended is re-armed every 500 ms, and every attempt re-resolves the
catalog. The refusals had power only because they denied the reconcile pass that would have ended
them — the first completed pass tears the suspended seats' tasks down, and a refusal needs a live
task.

The coupling is observable directly. Holding the recipient's resources/streams/resync/.lock
the exact flock the publication path takes — and then running a pass:

before   the pass had not returned after 20s; it completed only once the lock was released
after    the pass completes while the publication is still blocked

Goal

A reconcile pass completes whether or not resync publication is making progress, so no publication
condition can deny the pass that would resolve it.

Decisions

  • Publication moved to its own thread, rather than bounding the acknowledgement with a
    timeout. A timeout would leave the coupling in place and hide it behind a deadline; the
    acknowledgement exists to order watch installation against launches, which is a read, not a
    publication.
  • The worker stays the single writer of carrier state. The emitter reports outcomes back
    through the worker's own mailbox (Msg::Emitted) instead of touching baselines or deadlines.
  • One publication outstanding per subscription (Entry.in_flight). A reserved event identity
    must not be handed off twice while the outcome that decides whether it is spent has not returned.
  • Queued publications are cancelled when a subscription is deactivated or removed by a
    refresh, so nothing is published to a seat the pass has already decided receives no events. A
    publication the emitter already started is left to finish: it was in flight while the
    subscription was still active, and its outcome is discarded.
  • One re-arm mechanism. Re-observing the carrier after an outcome covers both a retained
    refusal and a carrier that moved on while the publication was outstanding. The first draft also
    set dirty and a deadline in the refusal branch; that was redundant, and while it was present
    it masked the re-observation clause under mutation (removing poll_paths was caught by no
    test). With the redundancy gone, that clause is caught by three.
  • The emitter is not joined at teardown. A publication blocked on another process's lock must
    not hold up supervisor teardown, and the emitter owns no state to hand back.
  • Terminal-refusal classification is deliberately not in this PR. It is still wanted — it
    removes the CPU burn and the unbounded retry — but on its own it would leave this coupling
    intact for the next condition that starves pass completion.

Verification

Fail-before / pass-afterrun::tests::reconcile_pass_completes_while_a_resync_publication_is_blocked
holds the recipient's stream lock, confirms the publication has not completed, then runs a pass. A
watchdog releases the lock only if the pass fails to finish on its own, and the test asserts the
watchdog did not have to:

before   FAILED in 22.73s — "the pass only completed after the blocked publication was released"
after    ok — 7 passed (this test plus the six existing resync pass tests) in 2.57s

Mutation — each shipped clause removed one at a time, full resync + resync-pass test selection
run each time. No survivors:

mutant caught by
publication back on the worker thread the pass test, plus the three queue tests
no single-outstanding-publication guard a_flush_never_hands_off_a_subscription_whose_publication_is_outstanding
deactivation does not cancel a queued publication deactivation_drops_only_that_recipients_queued_publication
refresh does not drop a removed subscription's publication a_refresh_drops_a_queued_publication_for_a_subscription_it_removed
outcome does not re-observe the carrier failed_tombstone_emit_retains_present_state_and_immutable_retry_snapshot + 2
published transition stays pending 7 existing tests
outcome leaves the subscription in flight 7 existing tests

The harness is self-checking: a run that does not compile, or that produces no test-result line, is
reported as UNRAN rather than as a surviving mutant. That guard earned itself — an earlier run of
the same matrix reported every mutant as surviving because cargo was not on the PATH of the
process that invoked it.

Gate — the exact test selection the Nix check runs (--lib --bins --test discovery --test codex_hooks --test hooks --test run --test driver_expansion, --test-threads=1) is green locally:
every target ok, lib 680 passed / 0 failed.

Suitecargo test -p st2 --lib: 680 passed, 0 failed. --test resync,
--test resync_notify_chain, --test event_e2e, --test reconcile, --test doctor: all green.
Every existing resync unit test still asserts exactly what it asserted before; they now drive the
worker and emitter halves in the order the loop runs them, through a #[cfg(test)] helper.
Production has one path.

docs/vrs/06-resync/spec.md gains the property this depends on: where publication runs, one
outstanding publication per subscription, and what happens to a queued or already-started
publication when a subscription goes away.

Complexity

One thread and one bounded queue. The queue exists because the two operations have genuinely
different liveness requirements: watch installation must answer a reconcile pass promptly, and
publication may block on another process. Merging them is what produced the defect. The alternative
— a mutex over carrier state released around each publication — needs the same outstanding/outcome
bookkeeping without the thread separation, so it is not simpler.

Concerns

  • Resync delivery is still serial. A publication blocked indefinitely now stalls resync
    delivery for every recipient behind it, though no longer any reconcile pass. This PR strictly
    improves on the previous behaviour, where the same condition also stalled the passes; refusal
    classification removes the case that made it likely.
  • A started publication can land just after a deactivation acknowledgement. Previously
    publication was synchronous on the worker thread, so ordering was total. The window is now
    bounded to a publication that was already in flight while the subscription was active. The
    existing invariant test for that boundary
    (dead_resync_seat_is_deactivated_before_its_relaunch_blocks) still passes.
  • The reconcile-pass stall of Terminal resync refusal is retried at 500 ms forever, re-resolving the catalog under the shared authoring lock and starving all writers #431 is not claimed to be fully explained by this change. That
    question stays open there, and the pass rate should be measured separately rather than assumed
    restored.

Friction & bottlenecks

  • INVARIANTS.md names three proofs in tests/resource_profile_supervisor_e2e.rs that no longer
    exist, so tests/invariants.rs::qualified_proof_references_resolve fails. It fails identically
    on pristine main (verified against origin/main's INVARIANTS.md), and it is outside the Nix
    gate's test selection, which is why it has stayed red. The correct repair needs the author's
    knowledge of which tests now prove that invariant, so it is not guessed at here.

  • tests/agent_publish.rs fails 9 of 22 with host 'host' must declare exactly one root agent; found 0. Reproduced with this branch's files reverted to the merge-base content, so it is
    pre-existing and unrelated. Also outside the Nix gate.

  • The devshell's Nix wrapper prints error (ignored): opening log file ... trace.jsonl: Permission denied on every invocation. Harmless, but it is the first line of every build log.

  • Overlaps PR Wake the supervisor promptly after catalog publication #433 on INVARIANTS.md and src/run.rs. Different regions — that PR adds a
    catalog-wakeup invariant row and touches src/watch.rs; this one appends a row and adds a test
    inside src/run.rs's existing test module. Whoever merges second resolves a trivial
    append-vs-append conflict in the table.

Follow-ups

References

Refs #431

Posted on behalf of @schickling
field value
agent_identity dev3.compoundingtech.st2.resync-lock.worker
session dev3.1788d1eb
agent_persona worker
agent_supervisor dev3.compoundingtech-lead
agent_tool OMP
agent_tool_version 18.1.2
agent_runtime OMP 18.1.2
tooling_profile dotfiles@7534055

A reconcile pass could not complete while resync publication was making no
progress. `install_live` and `deactivate` block on an acknowledgement from the
resync worker thread, and that same thread ran every publication — which takes
the shared catalog-authoring lock, re-resolves the catalog, and takes the
recipient's stream lock. So a publication that was slow, refused and retried, or
blocked on another process serialized the whole pass behind it, once per live
seat. That is the property that let a terminal-refusal loop deny reconcile
passes for two hours: the loop's only power was denying the pass that would have
ended it.

Publication now runs on its own thread. The worker captures a transition, hands
it off, and applies the outcome when it returns, so it stays the only writer of
carrier baselines and retry deadlines. One publication is outstanding per
subscription at a time; a queued publication is dropped when its subscription is
deactivated or removed by a refresh, so nothing is published to a seat the pass
has already decided receives no events. Re-observing the carrier after an
outcome is the single re-arm mechanism for both a retained refusal and a carrier
that moved on while the publication was outstanding.

This is the primary requirement from #431. Terminal-refusal classification is
separate and still wanted: it removes the CPU burn and the unbounded retry, but
it would leave this coupling intact for the next condition that starves pass
completion.

Refs #431

agent-identity: dev3.compoundingtech.st2.resync-lock.worker
agent-persona: worker
agent-supervisor: dev3.compoundingtech-lead
agent-tool: OMP
agent-tool-version: 18.1.2
agent-runtime: OMP 18.1.2
tooling-profile: dotfiles@7534055
The spec described the pass's acknowledged watch-set upsert and the retry
replay, but not which thread publishes. That is the property a pass's ability
to complete depends on, so it belongs in the spec rather than only in the
code.

Refs #431

agent-identity: dev3.compoundingtech.st2.resync-lock.worker
agent-persona: worker
agent-supervisor: dev3.compoundingtech-lead
agent-tool: OMP
agent-tool-version: 18.1.2
agent-runtime: OMP 18.1.2
tooling-profile: dotfiles@7534055
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