keeper: send node_active reports to monitor during graceful shutdown#1146
Merged
Conversation
On SIGTERM, the node-active service's main loop exits immediately, before calling keeper_update_pg_state() or service_keeper_node_active(). This leaves the monitor unaware that node1 is going offline until its next health-check poll, causing the flaky behaviour seen in ensure::test_004_demoted where node2 never reached wait_primary within the test timeout. Add keeper_node_active_shutdown_loop(): after the main loop breaks on asked_to_stop, report the current Postgres state to the monitor every 1s for up to 30 seconds (or until Postgres stops or an escalated signal arrives). This runs concurrently with the postgres-controller service calling 'pg_ctl stop -m fast': fast mode sends SIGTERM to the postmaster, which stops accepting new connections immediately, while the checkpoint completes asynchronously. By the time the first shutdown report fires, Postgres is already refusing connections; the monitor can start failover right away rather than waiting for a health-check timeout. SIGINT (asked_to_stop_fast) and SIGQUIT (asked_to_quit) skip the shutdown loop and exit immediately, as before.
test_003_init_secondary stops postgres on node2 then waits for it to recover as secondary. The original Python test (node2.stop_postgres()) sends SIGTERM directly to postgres without touching pg_autoctl's service controller, so the keeper auto-restarts postgres. The pgaftest port used 'stop postgres node2' which calls 'pg_autoctl manual service pgctl off' — this both stops postgres AND disables auto-restart. As a result node2's postgres stayed stopped for the rest of the spec. In test_004_demoted, NodeIsHealthy(node2) checks pgIsRunning, which was false because node2's postgres was never restarted. The failover transition at group_state_machine.c:550 requires NodeIsHealthy(activeNode) to be true, so the failover never triggered and the test timed out at 180s. Fix: add 'start postgres node2' immediately after 'stop postgres node2' to call 'pg_autoctl manual service pgctl on', re-enabling the keeper's auto-restart. The keeper restarts postgres and node2 converges back to secondary before test_004 runs.
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
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.
When the keeper process (node-active service) receives SIGTERM, it previously
broke out of its main loop immediately — before calling
keeper_update_pg_state()orservice_keeper_node_active(). The monitorhad no way to learn the node was going offline until its next health-check
poll, which could be several seconds later. During that window the monitor
may not trigger failover fast enough, causing standbys to time out waiting
for
wait_primary.Changes
service_keeper.c—keeper_node_active_shutdown_loop()After the main loop breaks on
asked_to_stop, the keeper reports itscurrent Postgres state to the monitor every 1 s for up to 30 seconds, or
until Postgres stops or an escalated signal (
SIGINT/SIGQUIT) arrives —whichever comes first. This runs concurrently with the postgres-controller
service calling
pg_ctl stop -m fast.Fast mode sends SIGTERM to the PostgreSQL postmaster. Inside
process_pm_shutdown_request()(src/backend/postmaster/postmaster.c) thepostmaster sets
Shutdown = FastShutdownand transitions toPM_STOP_BACKENDS. After that,canAcceptConnections()returnsCAC_SHUTDOWNfor every new connection attempt — the primary stopsaccepting writes before a single backend has rolled back, and long before the
checkpoint finishes. By the time the first shutdown report fires, Postgres
is already refusing connections; the monitor can start failover right away
rather than waiting for a health-check timeout.
SIGINT(asked_to_stop_fast) andSIGQUIT(asked_to_quit) skip theshutdown loop and exit immediately, as before.
Compatibility with PID-1 environments:
docker compose stopsends SIGTERM to PID 1, waitsstop_grace_period(default 10 s), then SIGKILL. The shutdown looptypically reports
pgIsRunning=falsewithin 1–2 s.terminationGracePeriodSeconds(default 30 s). The 30-second loop capaligns with this default.
systemctl stopsends SIGTERM;TimeoutStopSecdefaults to90 s. The loop exits as soon as Postgres stops, well before that limit.
In all three environments the protocol is identical: SIGTERM → bounded wait
→ SIGKILL. This change makes pg_auto_failover a cooperative participant
rather than exiting silently on the first signal.
tests/tap/specs/ensure.pgaf— fixtest_003_init_secondarystop postgres node2callspg_autoctl manual service pgctl off, whichstops Postgres and disables auto-restart. With auto-restart disabled,
node2->pgIsRunningstayed false throughtest_004_demoted, causingNodeIsHealthy(node2)to return false and the failover gate atgroup_state_machine.c:550to never fire.Fix: add
start postgres node2immediately after to re-enable the keeper'sauto-restart, matching the behaviour of the original Python test
(
node2.stop_postgres()sends SIGTERM to Postgres without touching theservice controller).