diff --git a/src/bin/common/pgsetup.c b/src/bin/common/pgsetup.c index c9e0b0ecf..a7bbd53ad 100644 --- a/src/bin/common/pgsetup.c +++ b/src/bin/common/pgsetup.c @@ -907,8 +907,12 @@ pg_setup_is_running(PostgresSetup *pgSetup) /* - * pg_setup_is_ready returns true when the postmaster.pid file has a "ready" - * status in it, which we parse in pgSetup->pm_status. + * pg_setup_is_ready returns true when the postmaster is accepting connections. + * That means pm_status "ready" (primary or promoted standby) or "standby" (hot + * standby still in streaming recovery). PostgreSQL writes "standby" to + * postmaster.pid when hot_standby=on and the instance is accepting read-only + * connections but has not yet been promoted; pg_ctl -w treats both statuses as + * "server started", and so do we. */ bool pg_setup_is_ready(PostgresSetup *pgSetup, bool pgIsNotRunningIsOk) @@ -935,7 +939,7 @@ pg_setup_is_ready(PostgresSetup *pgSetup, bool pgIsNotRunningIsOk) /* * Sometimes `pg_ctl start` returns with success and Postgres is still * in crash recovery replaying WAL files, in the "starting" state - * rather than the "ready" state. + * rather than the "ready" or "standby" state. * * In that case, we wait until Postgres is ready for connections. The * whole pg_autoctl code is expecting to be able to connect to @@ -945,7 +949,8 @@ pg_setup_is_ready(PostgresSetup *pgSetup, bool pgIsNotRunningIsOk) * ERROR Connection to database failed: FATAL: the database system is * starting up */ - while (pgSetup->pm_status != POSTMASTER_STATUS_READY) + while (pgSetup->pm_status != POSTMASTER_STATUS_READY && + pgSetup->pm_status != POSTMASTER_STATUS_STANDBY) { int maxRetries = 5; @@ -996,7 +1001,8 @@ pg_setup_is_ready(PostgresSetup *pgSetup, bool pgIsNotRunningIsOk) } /* avoid an extra wait if that's possible */ - if (pgSetup->pm_status == POSTMASTER_STATUS_READY) + if (pgSetup->pm_status == POSTMASTER_STATUS_READY || + pgSetup->pm_status == POSTMASTER_STATUS_STANDBY) { break; } @@ -1013,7 +1019,8 @@ pg_setup_is_ready(PostgresSetup *pgSetup, bool pgIsNotRunningIsOk) log_trace("pg_setup_is_ready: %s", pmStatusToString(pgSetup->pm_status)); } - return pgSetup->pm_status == POSTMASTER_STATUS_READY; + return pgSetup->pm_status == POSTMASTER_STATUS_READY || + pgSetup->pm_status == POSTMASTER_STATUS_STANDBY; } @@ -1091,14 +1098,15 @@ pg_setup_wait_until_is_ready(PostgresSetup *pgSetup, int timeout, int logLevel) *pgSetup = newPgSetup; /* avoid an extra pg_setup_is_ready call if we're all good already */ - pgIsReady = pgSetup->pm_status == POSTMASTER_STATUS_READY; + pgIsReady = pgSetup->pm_status == POSTMASTER_STATUS_READY || + pgSetup->pm_status == POSTMASTER_STATUS_STANDBY; } /* * Ok so we have a postmaster.pid file with a pid > 0 (not a standalone * backend, the service has started). Postgres might still be "starting" - * rather than "ready" though, so let's continue our attempts and make sure - * that Postgres is ready. + * rather than "ready" or "standby" though, so let's continue our attempts + * and make sure that Postgres is ready. */ for (; !pgIsReady; attempts++) { diff --git a/src/bin/pg_autoctl/fsm_transition.c b/src/bin/pg_autoctl/fsm_transition.c index 25698c790..c0bbf021e 100644 --- a/src/bin/pg_autoctl/fsm_transition.c +++ b/src/bin/pg_autoctl/fsm_transition.c @@ -1059,7 +1059,21 @@ fsm_prepare_for_secondary(Keeper *keeper) { LocalPostgresServer *postgres = &(keeper->postgres); - /* first. check that we're on the same timeline as the new primary */ + /* + * Verify that Postgres is accepting connections before proceeding. + * Postgres was started in the WAIT_STANDBY -> CATCHINGUP transition, but + * local_postgres_wait_until_ready may have returned as soon as the PID + * file appeared, before the postmaster finished startup recovery and began + * accepting connections. Calling ensure_postgres_service_is_running here + * re-checks readiness (via pg_setup_wait_until_is_ready) and closes that + * race window before we attempt SQL against the local instance. + */ + if (!ensure_postgres_service_is_running(postgres)) + { + return false; + } + + /* check that we're on the same timeline as the new primary */ if (!standby_check_timeline_with_upstream(postgres)) { /* errors have already been logged */ diff --git a/src/bin/pg_autoctl/primary_standby.c b/src/bin/pg_autoctl/primary_standby.c index cafcf8dc4..e9c0921ac 100644 --- a/src/bin/pg_autoctl/primary_standby.c +++ b/src/bin/pg_autoctl/primary_standby.c @@ -274,8 +274,16 @@ local_postgres_update(LocalPostgresServer *postgres, bool postgresNotRunningIsOk /* - * local_postgres_wait_until_ready waits until Postgres is running and updates - * our failure tracking counters for the Postgres service accordingly. + * local_postgres_wait_until_ready waits until Postgres is accepting + * connections and updates our failure tracking counters accordingly. + * + * We always go through pg_setup_wait_until_is_ready rather than short- + * circuiting on pg_is_running(). pg_is_running() (= pg_ctl status) returns + * true as soon as the postmaster PID file exists, which can happen before the + * postmaster has written PM_STATUS_READY or PM_STATUS_STANDBY to that file. + * Skipping the wait when the process already existed was the source of a race + * where a FSM transition could report a new state to the monitor while + * Postgres was still refusing TCP connections. */ static bool local_postgres_wait_until_ready(LocalPostgresServer *postgres) @@ -283,33 +291,26 @@ local_postgres_wait_until_ready(LocalPostgresServer *postgres) PostgresSetup *pgSetup = &(postgres->postgresSetup); int timeout = 10; /* wait for Postgres for 10s */ - bool pgIsRunning = pg_is_running(pgSetup->pg_ctl, pgSetup->pgdata); - log_trace("local_postgres_wait_until_ready: Postgres %s in \"%s\"", - pgIsRunning ? "is running" : "is not running", pgSetup->pgdata); - - if (!pgIsRunning) - { - /* main logging is done in the Postgres controller sub-process */ - pgIsRunning = pg_setup_wait_until_is_ready(pgSetup, timeout, LOG_DEBUG); + /* main logging is done in the Postgres controller sub-process */ + bool pgIsRunning = + pg_setup_wait_until_is_ready(pgSetup, timeout, LOG_DEBUG); - /* update connection string for connection to postgres */ - (void) - local_postgres_update_pg_failures_tracking(postgres, pgIsRunning); + /* update connection string for connection to postgres */ + (void) local_postgres_update_pg_failures_tracking(postgres, pgIsRunning); - if (pgIsRunning) - { - /* update pgSetup cache with new Postgres pid and all */ - local_postgres_init(postgres, pgSetup); + if (pgIsRunning) + { + /* update pgSetup cache with new Postgres pid and all */ + local_postgres_init(postgres, pgSetup); - log_debug("local_postgres_wait_until_ready: Postgres is running " - "with pid %d", pgSetup->pidFile.pid); - } - else - { - log_error("Failed to ensure that Postgres is running in \"%s\"", - pgSetup->pgdata); - } + log_debug("local_postgres_wait_until_ready: Postgres is running " + "with pid %d", pgSetup->pidFile.pid); + } + else + { + log_error("Failed to ensure that Postgres is running in \"%s\"", + pgSetup->pgdata); } return pgIsRunning;