Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -485,5 +485,13 @@ jobs:
echo "TRAVIS_BUILD_DIR=$(pwd)" >> $GITHUB_ENV

- name: Run pytest / ${{ matrix.TEST }} (PG${{ matrix.PGVERSION }})
timeout-minutes: 15
# citus is the heaviest TEST value in this matrix (6 files, ~76 items,
# several multi-coordinator/multi-worker convergence waits) and has
# been seen to run right up against a 15-minute step timeout while
# still making forward progress, not stuck -- e.g. citus (PG14)
# timed out mid-suite on 2026-07-29 with ~4-5 minutes of the job's
# own 20-minute ceiling (see above) left completely unused. 18
# minutes gives real headroom while still leaving margin under the
# job-level timeout once checkout/download/docker-load are counted.
timeout-minutes: 18
run: make run-test-prebuilt
11 changes: 11 additions & 0 deletions docs/ref/pgaftest.rst
Original file line number Diff line number Diff line change
Expand Up @@ -488,6 +488,17 @@ Commands inside ``setup``, ``teardown``, and ``step`` blocks
wait until <node> state is <state> [timeout <N>s]
wait until primary, secondary [timeout <N>s]
wait until <node> stopped [timeout <N>s]
wait until <node> replays <source> [timeout <N>s]

``wait until <node> replays <source>`` captures ``<source>``'s current WAL
position (or, if ``<source>`` is itself a standby, its own last-replayed
position) once, at the moment the command runs, then polls ``<node>`` until
it has replayed at least that far. Use it right after a write on ``<source>``
and before reading that data back from ``<node>``, instead of racing a fixed
``sleep`` against replication catch-up -- especially right after a
promotion/failover, when a node's FSM state can flip to
``primary``/``secondary`` a moment before a subsequent write has actually
propagated.

**Assertions**

Expand Down
47 changes: 45 additions & 2 deletions src/bin/common/pgsql.c
Original file line number Diff line number Diff line change
Expand Up @@ -545,8 +545,51 @@ pgsql_open_connection(PGSQL *pgsql)
INSTR_TIME_SET_CURRENT(pgsql->retryPolicy.startTime);
INSTR_TIME_SET_ZERO(pgsql->retryPolicy.connectTime);

/* Make a connection to the database */
pgsql->connection = PQconnectdb(pgsql->connectionString);
if (pgsql->connectionType == PGSQL_CONN_MONITOR)
{
/*
* Bound how long a query already in flight when the network
* disappears without a graceful FIN/RST (a hard network partition,
* or `docker network disconnect` in our own test suite) can block
* on read(), instead of relying on the OS default TCP retransmission
* timeout (many minutes). PGCONNECT_TIMEOUT above only bounds the
* initial handshake, not an already-established connection.
*
* Two distinct mechanisms, both needed: keepalives only start
* probing once a connection has been fully idle (no unacknowledged
* data) for keepalives_idle seconds -- if the query's own bytes are
* still unacknowledged at the moment the network vanishes, plain
* data retransmission (governed by the kernel's tcp_retries2, not
* by our keepalive settings) takes over instead and keepalives never
* get a chance to apply. tcp_user_timeout bounds that unacknowledged
* -data case directly, regardless of which timer the kernel is
* currently running.
*
* dbname carries the full connection string/URI as given by the
* caller; expand_dbname=1 tells libpq to parse it as such rather
* than take it literally, so this works whichever form (URI or
* keyword=value) pgsql->connectionString happens to be in, without
* us having to parse and rebuild it here ourselves.
*/
const char *keywords[] = {
"dbname", "keepalives", "keepalives_idle",
"keepalives_interval", "keepalives_count",
"tcp_user_timeout", NULL
};
const char *values[] = {
pgsql->connectionString, "1", POSTGRES_MONITOR_KEEPALIVES_IDLE,
POSTGRES_MONITOR_KEEPALIVES_INTERVAL,
POSTGRES_MONITOR_KEEPALIVES_COUNT,
POSTGRES_MONITOR_TCP_USER_TIMEOUT, NULL
};

pgsql->connection = PQconnectdbParams(keywords, values, 1);
}
else
{
/* Make a connection to the database */
pgsql->connection = PQconnectdb(pgsql->connectionString);
}

/* Check to see that the backend connection was successfully made */
if (PQstatus(pgsql->connection) != CONNECTION_OK)
Expand Down
64 changes: 64 additions & 0 deletions src/bin/pg_autoctl/cli_config.c
Original file line number Diff line number Diff line change
Expand Up @@ -649,6 +649,15 @@ cli_config_set(int argc, char **argv)

/*
* cli_keeper_config_set sets the given option path to the given value.
*
* pg_autoctl.name and pg_autoctl.hostname are node metadata that the monitor
* also carries a copy of: pushing the change there is normally left to the
* running supervisor's own config-reload handling, which this one-shot
* command can only trigger asynchronously (a SIGHUP) and has no way to wait
* on. Push those two synchronously here instead, the same way "pg_autoctl
* set node metadata" already does, so that by the time this command returns
* the monitor is guaranteed to be caught up -- no polling or sleep needed by
* the caller.
*/
static void
cli_keeper_config_set(int argc, char **argv)
Expand All @@ -675,6 +684,36 @@ cli_keeper_config_set(int argc, char **argv)
exit(EXIT_CODE_BAD_ARGS);
}

bool isNodeMetadata = streq(argv[0], "pg_autoctl.name") ||
streq(argv[0], "pg_autoctl.hostname");

/*
* Capture the pre-change values now: keeper_config_set_setting()
* below both patches this exact file on disk and repopulates config
* from it, so config itself won't hold the "old" values anymore by
* the time we'd want to compare.
*/
KeeperConfig oldConfig = { 0 };

if (isNodeMetadata)
{
bool missingPgdataIsOk = true;
bool pgIsNotRunningIsOk = true;
bool monitorDisabledIsOk = true;

oldConfig = keeperOptions;

if (!keeper_config_read_file(&oldConfig,
missingPgdataIsOk,
pgIsNotRunningIsOk,
monitorDisabledIsOk))
{
log_fatal("Failed to read configuration file \"%s\"",
config.pathnames.config);
exit(EXIT_CODE_BAD_CONFIG);
}
}

if (!keeper_config_set_setting(&config,
argv[0],
argv[1]))
Expand All @@ -692,6 +731,31 @@ cli_keeper_config_set(int argc, char **argv)
exit(EXIT_CODE_BAD_CONFIG);
}

/*
* Push the name/hostname change to the monitor synchronously,
* unless this node runs with its monitor disabled, in which case
* there is nothing to synchronize with.
*/
if (isNodeMetadata && !oldConfig.monitorDisabled)
{
Keeper keeper = { 0 };

keeper.config = config;

if (!monitor_init(&(keeper.monitor), config.monitor_pguri))
{
/* errors have already been logged */
exit(EXIT_CODE_BAD_ARGS);
}

if (!keeper_set_node_metadata(&keeper, &oldConfig))
{
log_error("Failed to update \"%s\" on the monitor, "
"see above for details", argv[0]);
exit(EXIT_CODE_MONITOR);
}
}

/* now read the value from just written file */
if (keeper_config_get_setting(&config,
argv[0],
Expand Down
11 changes: 9 additions & 2 deletions src/bin/pg_autoctl/config.c
Original file line number Diff line number Diff line change
Expand Up @@ -122,8 +122,15 @@ build_xdg_path(char *dst,
join_path_components(filename, filename, pgdata);
}

/* mkdir -p the target directory */
if (pg_mkdir_p(filename, 0755) == -1)
/*
* mkdir -p the target directory. Several pg_autoctl processes (e.g. a
* freshly started node's own supervisor and a one-shot CLI command
* invoked against it) can race to create the same XDG_RUNTIME_DIR
* fallback ancestor (typically "/tmp/pg_autoctl") the first time either
* of them runs -- tolerate losing that race rather than failing, as
* long as the directory exists by the time we're done.
*/
if (pg_mkdir_p(filename, 0755) == -1 && !directory_exists(filename))
{
log_error("Failed to create state directory \"%s\": %m", filename);
return false;
Expand Down
22 changes: 22 additions & 0 deletions src/bin/pg_autoctl/defaults.h
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,28 @@
#define MAXIMUM_BACKUP_RATE "100M"
#define MAXIMUM_BACKUP_RATE_LEN 32

/*
* TCP keepalives for the keeper-to-monitor connection. PGCONNECT_TIMEOUT
* only bounds the initial TCP handshake -- once connected, a query that's
* in flight when the network vanishes without a graceful FIN/RST (e.g. a
* hard `docker network disconnect`, or a real network partition) blocks on
* a plain read() with no libpq-level timeout of its own, relying entirely
* on the OS's default TCP retransmission timeout (many minutes). These
* values bound that worst case to keepalives_idle + keepalives_interval *
* keepalives_count =~ 11 seconds instead.
*/
#define POSTGRES_MONITOR_KEEPALIVES_IDLE "5"
#define POSTGRES_MONITOR_KEEPALIVES_INTERVAL "2"
#define POSTGRES_MONITOR_KEEPALIVES_COUNT "3"

/*
* Milliseconds. Bounds how long already-transmitted, unacknowledged data
* may go unacknowledged before the kernel gives up on the connection --
* the case plain keepalives don't cover (see the comment at the
* PQconnectdbParams() call site in pgsql_open_connection()).
*/
#define POSTGRES_MONITOR_TCP_USER_TIMEOUT "10000"


/*
* Microsoft approved cipher string.
Expand Down
7 changes: 7 additions & 0 deletions src/bin/pgaftest/cli_indent.c
Original file line number Diff line number Diff line change
Expand Up @@ -885,6 +885,13 @@ print_cmd(FILE *out, const TestCmd *cmd, int indent)
break;
}

case CMD_WAIT_LSN:
{
fformat(out, "%*swait until %s replays %s timeout %ds\n",
indent, "", cmd->service, cmd->state, cmd->timeoutSeconds);
break;
}

case CMD_PROMOTE:
{
fformat(out, "%*spromote", indent, "");
Expand Down
Loading