pgaftest/pg_autoctl: step-mode tooling (no-autopilot, fsm step, report/advance split) - #1183
Merged
Merged
Conversation
Introduces PG_AUTOCTL_STEP_MODE, an env var that switches the persistent node-active service into step mode: instead of ticking on its own, it blocks on a new Unix-domain control socket (<pidfile>.step) waiting for explicit "step" commands, and reuses the existing keeper_fsm_step() to advance the FSM by exactly one transition per command -- a gdb-step equivalent for the keeper FSM, useful for reproducing precise race-condition scenarios that outrun any external SQL-timed pgaftest script. `pg_autoctl manual fsm step` now detects the control socket and delegates to it instead of calling keeper_init() itself, which avoids reintroducing the status-file bug a fresh one-shot CLI process would otherwise cause (local_postgres_init() unlinks the shared pg_autoctl.pg file on every keeper_init(), and the persistent postgres subprocess only rewrites it when it actively needs to (re)start Postgres). Verified live in a 2-node Docker cluster: autopilot stays fully suspended between commands (no reported-state change over repeated polls despite a real pending transition), each step advances exactly one FSM edge (maintenance -> catchingup -> secondary), and a crashed Postgres is restarted on the next step command -- the same recovery path normal mode uses continuously, just triggered on-demand instead of every tick, which is the correct semantics for a mode whose whole point is that nothing happens without an explicit command. pgaftest's Makefile has its own curated SHARED_SRCS list (not a wildcard like pg_autoctl's own Makefile), so step_socket.c had to be added there explicitly for the pgaftest binary to link.
Adds two new .pgaf DSL primitives so a spec can precisely single-step a
node's FSM, the same way this session's earlier PG_AUTOCTL_STEP_MODE
facility (src/bin/pg_autoctl/step_socket.c) already lets an operator do
by hand:
- a per-node "no-autopilot" cluster modifier (test_spec.h's new
TestNode.noAutopilot, alongside launchDeferred/noMonitor) that emits
PG_AUTOCTL_STEP_MODE=1 in that node's compose environment
(compose_gen.c) -- its node-active service never ticks on its own.
- an "fsm step <node>" step-body command (new CMD_FSM_STEP), sugar for
`pg_autoctl manual fsm step --pgdata /var/lib/postgres/pgaf` run
inside the named node's container, modeled directly on the existing
"stop postgres <node>"/"start postgres <node>" sugar. Retries for up
to 30s on transient failure (test_runner.c): right after a step-mode
service (re)starts there's a brief handoff window with its sibling
postgres-controller subprocess during which a step can transiently
fail (e.g. postmaster.pid not observed yet) -- autopilot mode
self-heals this silently on the next tick, step mode has no next
tick unless retried.
While building the first live spec against this, found and fixed a real
gap in step mode itself: keeper_fsm_step() never refreshes the keeper's
cached list of other nodes, unlike the autonomous tick body
(service_keeper_node_active()). This went unnoticed because the only
other caller, the one-shot "pg_autoctl manual fsm step" CLI, gets a
fresh cache every invocation via its own fresh keeper_init() -- but
step mode's node-active service calls keeper_init() exactly once and
runs for its whole lifetime, so without an explicit refresh a peer that
registered after the service started would silently never get its
replication slot or HBA rule maintained. Fixed by refreshing the cache
before every step in service_keeper.c's step-mode branch, non-fatally
(falls through to the step regardless of refresh failure, matching how
a failed refresh in autonomous mode just retries next tick).
dimitri
added
the
Developer productivity
dimitri
force-pushed
the
pgaftest-step-mode-and-report-advance
branch
from
August 2, 2026 03:25
5aa9ff9 to
6446d3c
Compare
pg_autoctl manual fsm step reports the node's current state to the monitor and, in the same call, immediately attempts whatever transition the monitor assigns back -- atomically, with no way to observe (or hold a node frozen at) the moment in between. Add manual fsm step report / manual fsm step advance as the two independently-issuable halves of that combined call: - report calls monitor_node_active() and persists the assigned goal state without attempting the transition. - advance attempts the transition already on file (via keeper_fsm_reach_assigned_state) without a monitor round trip. Plain "step" keeps its original combined behavior unchanged. When a node's node-active service is running under PG_AUTOCTL_STEP_MODE (step mode), it owns the keeper's FSM instead of a one-shot CLI invocation, so step/report/advance are dispatched over a small Unix-domain-socket protocol (step_socket.c/h) to the running service rather than run standalone.
…t, fsm step Add fsm_step_report_advance.pgaf: a live spec proving the report/advance split works end to end using a no-autopilot node, registered in the default schedule and the node.sch CI schedule. Document the three new pieces of tooling this branch introduces: - the no-autopilot cluster node modifier - the pgaftest "fsm step <node>" DSL command - the "pg_autoctl manual fsm step [report|advance]" CLI split in docs/ref/pgaftest.rst (new "Step mode: no-autopilot nodes" section, node modifiers table, DSL command reference, test catalog entry) and docs/ref/pg_autoctl_manual.rst (fsm step synopsis).
dimitri
force-pushed
the
pgaftest-step-mode-and-report-advance
branch
from
August 2, 2026 03:30
6446d3c to
7c34bc7
Compare
The report/advance split had been documented as a paragraph spliced into the middle of pg_autoctl_manual.rst's literal command-tree output, breaking it into two disconnected fragments. Move that explanation into its own page, pg_autoctl_manual_fsm_step.rst, following the pattern already used for pg_autoctl_manual_service_restart.rst (Synopsis/Description/Options/ Examples), and link it from the manual page's toctree instead. Update pgaftest.rst's own cross-reference to point at the new page.
PGDATA is already set in every node container's environment (compose_gen.c), the same way every other "exec <node> pg_autoctl ..." command in the test suite already relies on it implicitly. Passing --pgdata /var/lib/postgres/pgaf explicitly added nothing and was inconsistent with the rest of the codebase; drop it from the fsm_step_report_advance.pgaf spec and from the "fsm step <node>" DSL command's own generated invocation (test_runner.c, plus the matching doc comment in test_spec_parse.y).
keeper_node_active_loop() had grown a stepMode boolean threaded through
its setup, its sleep/wait dispatch, and a large "if (stepMode) { ...;
continue; }" block sitting in the middle of the ordinary autopilot
body -- two genuinely different loops interleaved in one function via
a flag checked in three separate places.
Extract the step-mode behavior into its own keeper_step_mode_loop(),
called instead of keeper_node_active_loop() at the one call site
(cli_do_service.c) when PG_AUTOCTL_STEP_MODE is set. The "node
previously dropped from the monitor" check both loops need on startup
is factored into a shared keeper_exit_if_previously_dropped() helper.
keeper_node_active_loop() itself is otherwise unchanged: same
autopilot behavior, just without the step-mode branching that never
applied to it in practice (a step-mode node never reaches any of that
code -- it always hits the step-mode block and continues).
No behavior change: verified live against fsm_step_report_advance.pgaf
(step mode) and basic_operation.pgaf (28 tests, ordinary autopilot
path), both passing.
…ing) Renames the pgaftest cluster node modifier from "no-autopilot" to "suspended" everywhere: lexer token, grammar rule, AST field (TestNode.suspended), pretty-printer, and the two live specs. The backing environment variable PG_AUTOCTL_STEP_MODE becomes PG_AUTOCTL_SUSPENDED, and the pg_autoctl-side function that runs a suspended node's main loop is renamed keeper_suspended_loop (previously keeper_step_mode_loop). Comments and log messages throughout service_keeper.c, cli_do_fsm.c, cli_do_service.c, and step_socket.c are updated to say "suspended"/"suspended-node" instead of "step mode"/"step-mode", since that's the same concept under a clearer name -- inspired by the state debuggers (LLDB, Visual Studio, Unix job control) report for a process halted at a breakpoint, rather than the double-negative "no-autopilot". Documentation (pgaftest.rst's node-modifier table and "Suspended nodes" section, pg_autoctl_manual_fsm_step.rst) and the fsm_step_report_advance.pgaf spec are updated to match. Bison/flex outputs (test_spec_parse.c/h, test_spec_scan.c) regenerated from their .y/.l sources. No behavior change: verified live against fsm_step_report_advance.pgaf (4/4 passing) and a pretty-printer round-trip (pgaftest show spec) confirming the new keyword parses and re-emits correctly.
Verified the whole literal command-tree block against the actual built binary's --help output for every level under "pg_autoctl manual". Two of the mismatches were pre-existing (predate this branch, unrelated to the fsm step / suspended work): - "manual service pgctl" description said "...postgres controller", the real registered description says "...postgres service". - "manual primary adduser" capitalized "Add" where the real output is lowercase "add" for both monitor/replica lines. - "manual coordinator" and "manual coordinator update" descriptions were entirely reworded paraphrases; replaced with the actual registered CommandLine descriptions (which reference the real "master_update_node" Citus function by name). The "manual fsm"/"manual fsm nodes"/"manual service"/"manual service restart"/"manual"/"manual monitor"/"manual primary"/"manual primary slot"/"manual standby" sections were already verified accurate and left unchanged.
…-help "step" is both a terminal command (bare invocation runs the combined report+transition behavior) and effectively a parent for its "report"/ "advance" positional argument, but was registered as a plain make_command() leaf, so it never showed the "+" marker in "pg_autoctl manual fsm"'s own listing, and "pg_autoctl manual fsm step --help" never expanded into a sub-command list the way "nodes" does for "get"/"set". Fix: give the CommandLine node (built from a raw struct literal since the make_command()/make_command_set() macros are mutually exclusive between "run" and "subcommands") a subcommands array listing "report" and "advance". commandline_run() checks command->run before ever consulting subcommands, so real dispatch is completely unaffected -- this only feeds the two existing help-printing code paths (commandline_pretty_print_subcommands's "+" check, and commandline_print_usage's subcommand expansion). The report/advance entries' own "run" still points at cli_do_fsm_step for defensive consistency, though normal dispatch never reaches them. Verified: "pg_autoctl manual fsm" now shows "+ step", "pg_autoctl manual fsm step --help" now lists report/advance, and the live fsm_step_report_advance.pgaf spec still passes 4/4 (real report/ advance dispatch unchanged). docs/ref/pg_autoctl_manual_fsm_step.rst's Synopsis updated to match the new --help output.
Follow-up to the cli_do_fsm.c change that gives "step" a (decorative) subcommands array so the CLI's own --help output marks it with "+" and expands report/advance -- the literal command-tree block in this page still showed the old, unmarked "step" line.
pg_autoctl_manual.rst's convention expands every "+"-marked entry into its own "pg_autoctl manual X Y" block further down the same page (e.g. "fsm nodes" gets one for get/set) -- the previous commit added the "+" marker on "step" but not this expansion block, so the page promised a further breakdown it didn't actually show. Add it, matching the real --help output verbatim.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
pg_autoctl's node-active service normally reports a node's current stateto the monitor and, in the same call, immediately attempts whatever
transition the monitor assigns back — atomically, with no way to observe,
or hold a node at, the moment in between. That's the right default for
normal operation, but it makes some things impossible: reproducing a
specific FSM interleaving in a test, or, for an operator doing manual
recovery, inspecting what the monitor decided before acting on it.
This PR adds the tooling to do that: freeze a node's FSM at a specific
reported state, inspect what the monitor assigns next, and advance one
transition at a time, on demand.
What's in this PR
pg_autoctl manual fsm step [report|advance]— splits the existingcombined
manual fsm stepinto two independently-issuable halves.reportsends the node's current state to the monitor and persistswhatever goal state comes back, without attempting the transition.
advanceattempts the transition already on file, without talking tothe monitor again. Plain
step(no argument) is unchanged.Step mode (
PG_AUTOCTL_STEP_MODE) — when set, a node's node-activeservice stops ticking its FSM on its own and instead opens a small
Unix-domain-socket server, waiting to be told exactly when to
STEP,REPORT, orADVANCE.pg_autoctl manual fsm step/report/advancetalk to this socket when it's present, so an external driver gets
precise, single-step control over an otherwise-live keeper process.
no-autopilot— apgaftestcluster node modifier that starts anode with step mode enabled, and
fsm step <node>— DSL sugar forthe combined
manual fsm stepcall against that node, for use in testspecs that need to drive a node's FSM by hand.
Why this is useful on its own
A
no-autopilotnode paired withfsm step,report, andadvancelets a test hold a node frozen at a specific reported state while other
nodes in the cluster keep moving, then advance it exactly when the
scenario calls for it — instead of racing a freely-ticking FSM against
sleeps and hoping the timing lines up. The same CLI split is availabledirectly against a running node for manual, step-by-step recovery outside
of any test.
Proof this works end to end
tests/tap/specs/fsm_step_report_advance.pgafis a new spec thatexercises the full split live: a
no-autopilotnode reports a stalestate, the test observes the monitor assign a new goal state without the
node moving, then
advanceperforms that transition, then a follow-upreportconfirms the monitor sees the result. Verified locally, 4/4passing:
Registered in
tests/tap/scheduleandtests/tap/schedules/node.sch.Docs
docs/ref/pgaftest.rst: new "Step mode: no-autopilot nodes" section,no-autopilotnode-modifier table entry,fsm step <node>DSL commandreference, test catalog entry.
docs/ref/pg_autoctl_manual.rst:fsm step [report|advance]synopsisand explanation.
Verification
PG_CONFIG=... make -C src/bin/pg_autoctl/make -C src/bin/pgaftest— clean build.make docker-check(citus_indent --check) andci/banned.h.sh— pass.sphinx-build -b html— no new warnings from either touched doc file.