Skip to content

fix(scheduler): abort the in-flight program cleanly on shutdown - #279

Closed
phsilva wants to merge 1 commit into
astroufsc:masterfrom
phsilva:graceful-shutdown
Closed

fix(scheduler): abort the in-flight program cleanly on shutdown#279
phsilva wants to merge 1 commit into
astroufsc:masterfrom
phsilva:graceful-shutdown

Conversation

@phsilva

@phsilva phsilva commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Second of two stacked PRs fixing the Ctrl+C shutdown cascade. Stacked on
#280
— the first commit here is that PR; review only the last commit
(fix(scheduler): abort the in-flight program cleanly on shutdown). I'll
rebase this branch once #280 lands, which will drop the shared commit.

Problem

With #280 in place the bus stays alive through shutdown, but the scheduler
still mishandled it: the running exposure was never aborted, the program was
recorded as an ordinary error, and the machine marched on to start the next
program mid-teardown. Four independent bugs:

  • the machine's run-loop condition skipped the SHUTDOWN branch when the state
    changed while it slept in BUSY, so executor.stop() never ran; the loop now
    takes one state snapshot per iteration with SHUTDOWN checked first
  • a finishing worker could overwrite SHUTDOWN with IDLE/OFF and resurrect the
    machine; SHUTDOWN is now sticky
  • the worker's preamble (site clock, slew-start wait, program_begin) ran
    outside any exception handler, so a dying proxy killed the thread unhandled;
    the whole body now sits under one try
  • Scheduler.__stop__ joined nothing; it now joins the machine and the program
    worker (10s each, warns on timeout) so the abort round-trip completes before
    instruments stop

Plus supporting changes: handlers re-raise BusDeadException /
ObjectNotFoundException instead of wrapping them as program errors;
executor.stop() always arms must_stop and survives a failing abort
round-trip; the arm/clear moved to the machine thread so an abort can never
race the worker to the flag; Machine.sleep() re-checks state under the
condition lock, closing a missed-wakeup race.

Exception semantics: BusDeadException always aborts the program and stops
the schedule. ObjectNotFoundException aborts only during STOP/SHUTDOWN —
otherwise it stays a program ERROR and the schedule continues, so a schedule
referencing an undeployed controller (e.g. sample-sched.yaml's autofocus /
autoguide actions) behaves exactly as before.

Interrupted programs keep finished=False and re-run from their first action
on the next start — no DB/schema changes.

Verification

  • 7 new tests in tests/chimera/controllers/test_scheduler_machine.py,
    including regressions for the skipped-SHUTDOWN-branch bug and the
    next-program-on-dead-bus march; full suite green locally (270 passed,
    9 skipped)
  • E2E on a live system: SIGINT mid-exposure → exposure aborted in ~2.5s,
    program marked ABORTED, instruments stopped in order, bus last, exit 0 with
    zero tracebacks; SIGINT while idle → exit 0; double SIGINT → forced exit 130

Notes for reviewers (out of scope here)

  • sample-sched.yaml PRG01 has autofocus/pointverify/autoguide actions whose
    controllers aren't in the default config → those actions fail as program
    ERRORs (pre-existing behavior, preserved)
  • sequential.done() logs every error with a full traceback, so even a clean
    abort prints a ProgramExecutionAborted stack — cosmetic, could be toned
    down later
  • the 10s join timeouts in Scheduler.__stop__ may want to be configurable
    for slow-readout cameras

@phsilva phsilva changed the title fix: shut down gracefully on ctrl-c, aborting in-flight programs fix(scheduler): abort the in-flight program cleanly on shutdown Jul 28, 2026
@wschoenell

Copy link
Copy Markdown
Contributor

Heads up: this overlaps substantially with #255 in machine.py/executor.py — both PRs fix the "scheduler won't stop / worker outlives the state machine" family, from two different incidents (#255 from four live nights on the LNA 40 cm STOP path, this one from the Ctrl+C SHUTDOWN path). Mapping the overlap so we can converge instead of conflict:

Same bug, both PRs, different mechanisms:

Bug #255 #279
A finishing worker resurrects a stopped machine compare-and-set terminal transitions gated on BUSY (transition(State.BUSY, ...)) — covers STOP and SHUTDOWN, including the stop-then-program-error → IDLE re-arm we hit live on 2026-07-25 state() ignores changes once SHUTDOWN — covers SHUTDOWN only
SHUTDOWN branch skipped when the state changes mid-sleep shutdown cleanup moved after the run loop, so it runs on any exit path snapshot-per-iteration loop with SHUTDOWN checked first
stop() with no action in flight never arms the flag must_stop.set() unconditionally in stop() identical

Unique to #255 (already merged with current master, suite green): STOPPING state with OFF gated on actual worker death + heartbeat, duplicate-worker guard, start_at ordering, stale-queue re-check, expired programs, inline tracking stop, an end-to-end stop regression rig.

Unique here (and wanted!): the whole preamble-under-one-try + BusDead/ObjectNotFound taxonomy (#255's own tests just reproduced exactly that unhandled-preamble death when master's get_site() landed under them), handlers re-raising shutdown-type exceptions, abort-round-trip failure tolerated in stop(), must_stop arm/clear on the machine thread, sleep(until_leaves=...), __stop__ joining machine+worker, daemon threads — plus #280 underneath it all.

Proposed order: #280 first (CLI-only, orthogonal) → #255 (7 field-tested commits, currently mergeable) → this PR rebased on the result, where it shrinks to its unique items with two adaptations: the new exception paths use transition(State.BUSY, ...) instead of unconditional state(IDLE)/state(OFF), and the sticky-SHUTDOWN guard in state() is still worth keeping (it also blocks a late external start() after shutdown, which the CAS doesn't).

One semantic split worth stating explicitly in the merged result: STOP waits for the worker indefinitely with a heartbeat (an operator asked; OFF must be honest — on 2026-07-25 a premature OFF hid 52 min of exposures on a closed dome), while SHUTDOWN joins bounded at 10 s with daemon threads (the process must be able to exit). They're different paths and compose fine. Also happy to rename #255's _worker to current_worker to match __stop__'s access here.

Happy to do the #255-side adaptations; review of the stop semantics there welcome.

Four independent bugs made shutdown-while-observing messy:

- the machine's run loop condition skipped the SHUTDOWN branch when the
  state changed while it slept in BUSY, so executor.stop() never ran and
  the exposure was left to die with the bus. Restructure to one state
  snapshot per iteration with SHUTDOWN checked first.
- a finishing worker could overwrite SHUTDOWN with IDLE/OFF and
  resurrect the machine; SHUTDOWN is now sticky.
- the worker's pre-try preamble (site clock, program wait loop) ran
  outside any handler, so a proxy dying there killed the thread
  unhandled. The whole body now sits under one try, and shutdown-type
  exceptions get distinct semantics: BusDeadException always aborts the
  program and stops the schedule; ObjectNotFoundException aborts only
  during STOP/SHUTDOWN and is an ordinary program error otherwise, so a
  schedule referencing an undeployed controller keeps going as before.
- Scheduler.__stop__ joined nothing; it now joins the machine and the
  program worker (10s each) so the abort round-trip completes before
  instruments stop.

Handlers re-raise BusDeadException/ObjectNotFoundException instead of
wrapping them as ProgramExecutionException; executor.stop() always arms
must_stop and survives an abort round-trip failing; the must_stop
arm/clear moved to the machine thread so an abort can never race the
worker to the flag. Machine.sleep() re-checks state under the condition
lock, closing a missed-wakeup race.
@phsilva
phsilva force-pushed the graceful-shutdown branch from 6d4243a to 1748f70 Compare July 29, 2026 20:52
@phsilva
phsilva marked this pull request as draft July 29, 2026 20:54
@phsilva phsilva closed this Jul 29, 2026
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