diff --git a/src/bin/common/pgsql.c b/src/bin/common/pgsql.c index f618c2cb4..fe36847f4 100644 --- a/src/bin/common/pgsql.c +++ b/src/bin/common/pgsql.c @@ -50,6 +50,7 @@ static bool pgsql_alter_system_set(PGSQL *pgsql, GUC setting); static bool pgsql_get_current_setting(PGSQL *pgsql, char *settingName, char **currentValue); static void parsePgMetadata(void *ctx, PGresult *result); +static void parsePgVersion(void *ctx, PGresult *result); static void parsePgReachedTargetLSN(void *ctx, PGresult *result); static void parseReplicationSlotMaintain(void *ctx, PGresult *result); static void parsePgReachedTargetLSN(void *ctx, PGresult *result); @@ -2778,6 +2779,118 @@ parsePgMetadata(void *ctx, PGresult *result) } +/* + * PgVersionInfo carries the parsed result of pgsql_get_postgres_version's + * query. Deliberately not PostgresVersionInfo (primary_standby.h): that + * struct also carries needsReport, a keeper-side bookkeeping field this + * generic pgsql.c layer has no business setting. + */ +typedef struct PgVersionInfo +{ + char sqlstate[6]; + bool parsedOk; + int versionNum; + char version[NAMEDATALEN]; + char versionString[BUFSIZE]; + char citusVersion[NAMEDATALEN]; +} PgVersionInfo; + + +/* + * pgsql_get_postgres_version fetches the connected server's own version + * (server_version_num/server_version/version()) and, when installed, the + * Citus extension's version. Unlike pgsql_get_postgres_metadata, none of + * this can change without a Postgres restart, so callers should fetch it + * once per restart rather than on every periodic report. + */ +bool +pgsql_get_postgres_version(PGSQL *pgsql, + int *versionNum, + char *version, + char *versionString, + char *citusVersion) +{ + PgVersionInfo context = { 0 }; + + char *sql = + "select current_setting('server_version_num')::int," + " current_setting('server_version')," + " version()," + " (select extversion from pg_extension where extname = 'citus')"; + + if (!pgsql_execute_with_params(pgsql, sql, 0, NULL, NULL, + &context, &parsePgVersion)) + { + /* errors have been logged already */ + return false; + } + + if (!context.parsedOk) + { + log_error("Failed to parse the Postgres version"); + return false; + } + + *versionNum = context.versionNum; + strlcpy(version, context.version, NAMEDATALEN); + strlcpy(versionString, context.versionString, BUFSIZE); + strlcpy(citusVersion, context.citusVersion, NAMEDATALEN); + + pgsql_finish(pgsql); + + return true; +} + + +/* + * parsePgVersion parses the result from pgsql_get_postgres_version's query: + * server_version_num, server_version, version(), and Citus's extversion + * (NULL when Citus isn't installed). + */ +static void +parsePgVersion(void *ctx, PGresult *result) +{ + PgVersionInfo *context = (PgVersionInfo *) ctx; + char *value; + + if (PQnfields(result) != 4) + { + log_error("Query returned %d columns, expected 4", PQnfields(result)); + context->parsedOk = false; + return; + } + + if (PQntuples(result) != 1) + { + log_error("Query returned %d rows, expected 1", PQntuples(result)); + context->parsedOk = false; + return; + } + + value = PQgetvalue(result, 0, 0); + if (!stringToInt(value, &(context->versionNum))) + { + log_error("Failed to parse server_version_num \"%s\"", value); + context->parsedOk = false; + return; + } + + strlcpy(context->version, PQgetvalue(result, 0, 1), NAMEDATALEN); + strlcpy(context->versionString, PQgetvalue(result, 0, 2), BUFSIZE); + + if (PQgetisnull(result, 0, 3)) + { + context->citusVersion[0] = '\0'; + } + else + { + strlcpy(context->citusVersion, PQgetvalue(result, 0, 3), NAMEDATALEN); + } + + context->parsedOk = true; +} + + typedef struct PgReachedTargetLSN { char sqlstate[6]; diff --git a/src/bin/common/pgsql.h b/src/bin/common/pgsql.h index 173811e88..b8f46a240 100644 --- a/src/bin/common/pgsql.h +++ b/src/bin/common/pgsql.h @@ -389,6 +389,12 @@ bool pgsql_get_postgres_metadata(PGSQL *pgsql, char *pgsrSyncState, char *currentLSN, PostgresControlData *control); +bool pgsql_get_postgres_version(PGSQL *pgsql, + int *versionNum, + char *version, + char *versionString, + char *citusVersion); + bool pgsql_one_slot_has_reached_target_lsn(PGSQL *pgsql, char *targetLSN, char *currentLSN, diff --git a/src/bin/pg_autoctl/keeper.c b/src/bin/pg_autoctl/keeper.c index ee4997558..d74f7452f 100644 --- a/src/bin/pg_autoctl/keeper.c +++ b/src/bin/pg_autoctl/keeper.c @@ -435,6 +435,27 @@ keeper_update_pg_state(Keeper *keeper, int logLevel) bool pgIsNotRunningIsOk = true; + /* + * true iff *this function* already determined Postgres was running as + * of its own previous call. Compared against the freshly determined + * postgres->pgIsRunning further down to detect a not-running -> + * running edge -- the trigger for re-fetching version info that can't + * change without a Postgres restart (see PostgresVersionInfo). + * + * Deliberately NOT postgres->pgIsRunning's own previous value: that + * field is also written directly by fsm_transition.c/primary_standby.c + * during FSM transitions (e.g. the initial init -> single transition), + * outside of this function entirely. Comparing against those writes + * would make the very first, most important edge -- Postgres coming up + * for the first time after `pg_autoctl create`/`run` -- invisible, + * since pgIsRunning could already read true by the time this function + * next runs. postgres->pgVersion.lastKnownRunning is written only + * here, immediately below, so it always reflects this function's own + * last determination, regardless of what else touches pgIsRunning in + * between two of its calls. + */ + bool wasRunning = postgres->pgVersion.lastKnownRunning; + log_debug("Update local PostgreSQL state"); /* reinitialize the replication state values each time we update */ @@ -513,6 +534,34 @@ keeper_update_pg_state(Keeper *keeper, int logLevel) keeperState->pg_control_version = pgSetup->control.pg_control_version; keeperState->catalog_version_no = pgSetup->control.catalog_version_no; keeperState->system_identifier = pgSetup->control.system_identifier; + + /* + * Postgres just transitioned from not-running to running: fetch its + * version (and Citus's, if installed) once now, rather than on + * every periodic report -- neither can change without a Postgres + * restart. needsReport is cleared by keeper_node_active() once it's + * been successfully sent to the monitor; a failed fetch here simply + * leaves it unset, to be retried on the next such edge (i.e. the + * next Postgres restart) rather than every tick. + */ + if (!wasRunning && postgres->pgIsRunning) + { + PostgresVersionInfo *pgVersion = &(postgres->pgVersion); + + if (pgsql_get_postgres_version(pgsql, + &(pgVersion->versionNum), + pgVersion->version, + pgVersion->versionString, + pgVersion->citusVersion)) + { + pgVersion->needsReport = true; + } + else + { + log_level(logLevel, + "Failed to fetch Postgres/Citus version info"); + } + } } else { @@ -543,6 +592,14 @@ keeper_update_pg_state(Keeper *keeper, int logLevel) } } + /* + * Sync our own edge-tracker to what this function just determined, + * regardless of which branch above ran -- see the comment on + * wasRunning above for why this is deliberately not the same as + * postgres->pgIsRunning's own (multiply-written) value. + */ + postgres->pgVersion.lastKnownRunning = postgres->pgIsRunning; + /* * In some states, PostgreSQL isn't expected to be running, or not expected * to have a streaming replication to monitor at all. @@ -1305,16 +1362,42 @@ keeper_node_active(Keeper *keeper, bool doInit, /* * Report the current state to the monitor and get the assigned state. */ - return monitor_node_active(monitor, - config->formation, - keeperState->current_node_id, - keeperState->current_group, - keeperState->current_role, - reportPgIsRunning, - postgres->postgresSetup.control.timeline_id, - postgres->currentLSN, - postgres->pgsrSyncState, - assignedState); + bool nodeActiveReturned = + monitor_node_active(monitor, + config->formation, + keeperState->current_node_id, + keeperState->current_group, + keeperState->current_role, + reportPgIsRunning, + postgres->postgresSetup.control.timeline_id, + postgres->currentLSN, + postgres->pgsrSyncState, + assignedState); + + /* + * Postgres/Citus version info doesn't change without a Postgres + * restart, so it's reported separately from the per-tick node_active + * call above, only when keeper_update_pg_state() detected a fresh + * not-running -> running edge. Only attempt it once node_active itself + * succeeded, so we know the monitor connection is good; leave + * needsReport set on failure so the next tick retries. + */ + if (nodeActiveReturned && postgres->pgVersion.needsReport) + { + if (monitor_report_postgres_version(monitor, + keeperState->current_node_id, + &(postgres->pgVersion))) + { + postgres->pgVersion.needsReport = false; + } + else + { + log_warn("Failed to report Postgres/Citus version to the " + "monitor, will retry"); + } + } + + return nodeActiveReturned; } diff --git a/src/bin/pg_autoctl/monitor.c b/src/bin/pg_autoctl/monitor.c index 0d517ee1b..eb9a266c0 100644 --- a/src/bin/pg_autoctl/monitor.c +++ b/src/bin/pg_autoctl/monitor.c @@ -1150,6 +1150,51 @@ monitor_set_node_region(Monitor *monitor, } +/* + * monitor_report_postgres_version reports the connected Postgres server's + * own version and, when installed, the Citus extension's version, for the + * given node. Called once per Postgres restart (see + * keeper_update_pg_state()'s pgIsRunning edge detection), never on every + * periodic report -- neither piece of information can change without a + * Postgres restart. citusVersion may legitimately be an empty string + * (Citus not installed on this node), reported as SQL NULL. + */ +bool +monitor_report_postgres_version(Monitor *monitor, int64_t nodeId, + PostgresVersionInfo *pgVersion) +{ + PGSQL *pgsql = &monitor->pgsql; + const char *sql = + "SELECT pgautofailover.report_postgres_version($1, $2, $3, $4, $5)"; + + int paramCount = 5; + Oid paramTypes[5] = { INT8OID, INT4OID, TEXTOID, TEXTOID, TEXTOID }; + const char *paramValues[5]; + + IntString nodeIdString = intToString(nodeId); + IntString versionNumString = intToString(pgVersion->versionNum); + + paramValues[0] = nodeIdString.strValue; + paramValues[1] = versionNumString.strValue; + paramValues[2] = pgVersion->version; + paramValues[3] = pgVersion->versionString; + paramValues[4] = IS_EMPTY_STRING_BUFFER(pgVersion->citusVersion) + ? NULL + : pgVersion->citusVersion; + + if (!pgsql_execute_with_params(pgsql, sql, paramCount, paramTypes, + paramValues, NULL, NULL)) + { + log_error("Failed to report Postgres/Citus version for node %" PRId64, + nodeId); + + return false; + } + + return true; +} + + /* * monitor_get_node_region retrieves the region label of a node from the * monitor. diff --git a/src/bin/pg_autoctl/monitor.h b/src/bin/pg_autoctl/monitor.h index fc27dc6e3..8704f354b 100644 --- a/src/bin/pg_autoctl/monitor.h +++ b/src/bin/pg_autoctl/monitor.h @@ -163,6 +163,8 @@ bool monitor_set_node_region(Monitor *monitor, bool monitor_get_node_region(Monitor *monitor, char *name, char *region, size_t size); +bool monitor_report_postgres_version(Monitor *monitor, int64_t nodeId, + PostgresVersionInfo *pgVersion); bool monitor_get_formation_number_sync_standbys(Monitor *monitor, char *formation, int *numberSyncStandbys); bool monitor_set_formation_number_sync_standbys(Monitor *monitor, char *formation, diff --git a/src/bin/pg_autoctl/primary_standby.h b/src/bin/pg_autoctl/primary_standby.h index a03401a55..7da8b725d 100644 --- a/src/bin/pg_autoctl/primary_standby.h +++ b/src/bin/pg_autoctl/primary_standby.h @@ -25,6 +25,37 @@ typedef struct LocalExpectedPostgresStatus } LocalExpectedPostgresStatus; +/* + * PostgresVersionInfo holds the connected Postgres server's own version + * (server_version_num/server_version/version()) and, when installed, the + * Citus extension's version. Unlike pgsrSyncState/currentLSN, none of this + * can change without a Postgres restart, so it's fetched once per restart + * (see keeper_update_pg_state()'s pgIsRunning edge detection) rather than + * on every periodic report. needsReport stays true from the moment it's + * fetched until the next successful monitor_report_postgres_version() call. + */ +typedef struct PostgresVersionInfo +{ + int versionNum; /* server_version_num */ + char version[NAMEDATALEN]; /* server_version, e.g. "16.3" */ + char versionString[BUFSIZE]; /* version(), full descriptive string */ + char citusVersion[NAMEDATALEN]; /* citus extversion; "" when not installed */ + bool needsReport; + + /* + * lastKnownRunning is keeper_update_pg_state()'s own private edge + * tracker, exclusively written by that function -- deliberately NOT + * the same thing as LocalPostgresServer.pgIsRunning above, which other + * code (fsm_transition.c, primary_standby.c) also writes directly + * during FSM transitions. Comparing against a value only this + * function ever sets is what makes a not-running -> running edge + * reliably observable here, regardless of what else touches + * pgIsRunning in between two of this function's own calls. + */ + bool lastKnownRunning; +} PostgresVersionInfo; + + /* * LocalPostgresServer represents a local postgres database cluster that * we can manage via a SQL connection and operations on the database @@ -47,6 +78,7 @@ typedef struct LocalPostgresServer LocalExpectedPostgresStatus expectedPgStatus; char standbyTargetLSN[PG_LSN_MAXLENGTH]; char synchronousStandbyNames[BUFSIZE]; + PostgresVersionInfo pgVersion; } LocalPostgresServer; diff --git a/src/monitor/expected/node_active_protocol.out b/src/monitor/expected/node_active_protocol.out index 91989cfa2..1fb5b73c6 100644 --- a/src/monitor/expected/node_active_protocol.out +++ b/src/monitor/expected/node_active_protocol.out @@ -618,3 +618,55 @@ ERROR: node "unknown_node" is not registered in formation "fsm_test" -- empty region: error SELECT pgautofailover.set_node_region('fsm_test', 'node1', ''); ERROR: invalid value for region: expected a non-empty string +-- ── test_008: report_postgres_version ──────────────────────────────────────── +-- +-- Postgres/Citus version info is a plain self-report: not part of the FSM, +-- never queried by node_active, and (unlike set_node_region) doesn't error +-- on an unknown node -- a keeper reporting on its own already-known nodeid +-- is a harmless no-op if that row no longer exists, same as +-- ReportAutoFailoverNodeState()'s own report path. +-- never reported yet: all four columns are NULL +SELECT pg_versionnum, pg_version, pg_versionstring, citus_version + FROM pgautofailover.node WHERE nodeid = :n1; +-[ RECORD 1 ]----+- +pg_versionnum | +pg_version | +pg_versionstring | +citus_version | + +SELECT pgautofailover.report_postgres_version( + :n1, 170003, '17.3', 'PostgreSQL 17.3 on x86_64-linux', '12.1'); +-[ RECORD 1 ]-----------+- +report_postgres_version | + +SELECT pg_versionnum, pg_version, pg_versionstring, citus_version + FROM pgautofailover.node WHERE nodeid = :n1; +-[ RECORD 1 ]----+-------------------------------- +pg_versionnum | 170003 +pg_version | 17.3 +pg_versionstring | PostgreSQL 17.3 on x86_64-linux +citus_version | 12.1 + +-- citus_version omitted (defaults to NULL): a later report legitimately +-- clears a previously-reported citus_version, e.g. Citus was removed +SELECT pgautofailover.report_postgres_version( + :n1, 170003, '17.3', 'PostgreSQL 17.3 on x86_64-linux'); +-[ RECORD 1 ]-----------+- +report_postgres_version | + +SELECT pg_versionnum, pg_version, pg_versionstring, citus_version + FROM pgautofailover.node WHERE nodeid = :n1; +-[ RECORD 1 ]----+-------------------------------- +pg_versionnum | 170003 +pg_version | 17.3 +pg_versionstring | PostgreSQL 17.3 on x86_64-linux +citus_version | + +-- null node_id: error +SELECT pgautofailover.report_postgres_version(NULL, 170003); +ERROR: report_postgres_version requires a non-null node_id +-- unknown node_id: silent no-op, not an error +SELECT pgautofailover.report_postgres_version(-1, 170003); +-[ RECORD 1 ]-----------+- +report_postgres_version | + diff --git a/src/monitor/node_active_protocol.c b/src/monitor/node_active_protocol.c index a0d1cf128..459514e92 100644 --- a/src/monitor/node_active_protocol.c +++ b/src/monitor/node_active_protocol.c @@ -69,6 +69,7 @@ PG_FUNCTION_INFO_V1(stop_maintenance); PG_FUNCTION_INFO_V1(set_node_candidate_priority); PG_FUNCTION_INFO_V1(set_node_replication_quorum); PG_FUNCTION_INFO_V1(set_node_region); +PG_FUNCTION_INFO_V1(report_postgres_version); PG_FUNCTION_INFO_V1(synchronous_standby_names); PG_FUNCTION_INFO_V1(testing_lock_formation); PG_FUNCTION_INFO_V1(testing_lock_node_group); @@ -2494,6 +2495,53 @@ set_node_region(PG_FUNCTION_ARGS) } +/* + * report_postgres_version persists a node's Postgres server version and, + * when installed, Citus extension version. This is a plain self-report, + * called once by the keeper per Postgres restart (see pg_autoctl's + * keeper_update_pg_state()) -- not routed through node_active() since + * neither piece of information can change without a restart, and not + * requiring the node to exist (like set_node_region does for its + * operator-facing, named-node lookup): a keeper reporting on its own + * already-known nodeId that no longer exists is a harmless no-op, the same + * as ReportAutoFailoverNodeState()'s own report path. + */ +Datum +report_postgres_version(PG_FUNCTION_ARGS) +{ + checkPgAutoFailoverVersion(); + + if (PG_ARGISNULL(0)) + { + ereport(ERROR, + (errmsg("report_postgres_version requires a non-null " + "node_id"))); + } + + int64 nodeId = PG_GETARG_INT64(0); + + bool versionNumIsNull = PG_ARGISNULL(1); + int32 versionNum = versionNumIsNull ? 0 : PG_GETARG_INT32(1); + + char *version = + PG_ARGISNULL(2) ? NULL : text_to_cstring(PG_GETARG_TEXT_P(2)); + + char *versionString = + PG_ARGISNULL(3) ? NULL : text_to_cstring(PG_GETARG_TEXT_P(3)); + + char *citusVersion = + PG_ARGISNULL(4) ? NULL : text_to_cstring(PG_GETARG_TEXT_P(4)); + + ReportAutoFailoverNodeVersion(nodeId, + versionNumIsNull ? NULL : &versionNum, + version, + versionString, + citusVersion); + + PG_RETURN_VOID(); +} + + /* * update_node_metadata allows to update a node's nodename, hostname, and port. * diff --git a/src/monitor/node_metadata.c b/src/monitor/node_metadata.c index a40ccb4b4..9c1660b7e 100644 --- a/src/monitor/node_metadata.c +++ b/src/monitor/node_metadata.c @@ -1774,6 +1774,72 @@ ReportAutoFailoverNodeRegion(int64 nodeid, } +/* + * ReportAutoFailoverNodeVersion persists a node's Postgres server version + * and, when installed, Citus extension version. Unlike the region/state + * report functions, several arguments here are legitimately allowed to be + * NULL (citusVersion whenever Citus isn't installed; the whole trio of + * Postgres version fields, in principle, though the keeper only ever calls + * this once it has all three) -- pass NULL pointers for whichever of + * version/versionString/citusVersion aren't available, and NULL for + * versionNum itself to mean "no Postgres version to report". + * + * We use SPI to automatically handle triggers, function calls, etc. + */ +void +ReportAutoFailoverNodeVersion(int64 nodeid, + int *versionNum, + char *version, + char *versionString, + char *citusVersion) +{ + Oid argTypes[] = { + INT4OID, /* pg_versionnum */ + TEXTOID, /* pg_version */ + TEXTOID, /* pg_versionstring */ + TEXTOID, /* citus_version */ + INT8OID /* nodeid */ + }; + + Datum argValues[] = { + versionNum != NULL ? Int32GetDatum(*versionNum) : (Datum) 0, + version != NULL ? CStringGetTextDatum(version) : (Datum) 0, + versionString != NULL ? CStringGetTextDatum(versionString) : (Datum) 0, + citusVersion != NULL ? CStringGetTextDatum(citusVersion) : (Datum) 0, + Int64GetDatum(nodeid) + }; + + char argNulls[] = { + versionNum != NULL ? ' ' : 'n', + version != NULL ? ' ' : 'n', + versionString != NULL ? ' ' : 'n', + citusVersion != NULL ? ' ' : 'n', + ' ' + }; + + const int argCount = sizeof(argValues) / sizeof(argValues[0]); + + const char *updateVersionQuery = + "UPDATE " AUTO_FAILOVER_NODE_TABLE + " SET pg_versionnum = $1, pg_version = $2, " + " pg_versionstring = $3, citus_version = $4 " + " WHERE nodeid = $5"; + + SPI_connect(); + + int versionSpiStatus = SPI_execute_with_args(updateVersionQuery, + argCount, argTypes, argValues, + argNulls, false, 0); + + if (versionSpiStatus != SPI_OK_UPDATE) + { + elog(ERROR, "could not update " AUTO_FAILOVER_NODE_TABLE); + } + + SPI_finish(); +} + + /* * UpdateAutoFailoverNodeMetadata updates a node registration to a possibly new * nodeName, nodeHost, and nodePort. Those are NULL (or zero) when not changed. diff --git a/src/monitor/node_metadata.h b/src/monitor/node_metadata.h index 407de0a7d..ccc2eed57 100644 --- a/src/monitor/node_metadata.h +++ b/src/monitor/node_metadata.h @@ -238,6 +238,11 @@ extern void ReportAutoFailoverNodeRegion(int64 nodeid, char *nodeHost, int nodePort, char *region); +extern void ReportAutoFailoverNodeVersion(int64 nodeid, + int *versionNum, + char *version, + char *versionString, + char *citusVersion); extern void UpdateAutoFailoverNodeMetadata(int64 nodeid, char *nodeName, char *nodeHost, diff --git a/src/monitor/pgautofailover.sql b/src/monitor/pgautofailover.sql index ce2bda9e8..83ae1acb4 100644 --- a/src/monitor/pgautofailover.sql +++ b/src/monitor/pgautofailover.sql @@ -124,6 +124,17 @@ CREATE TABLE pgautofailover.node region text not null default 'default', replication_stall_since timestamptz, + -- Postgres/Citus version info, reported once per Postgres restart + -- (see pg_autoctl's keeper_update_pg_state) rather than on every + -- periodic report -- neither can change without a Postgres restart. + -- NULL until the first report; pg_versionnum/pg_version/ + -- pg_versionstring are always reported together, citus_version is + -- NULL whenever Citus isn't installed on that node. + pg_versionnum int, + pg_version text, + pg_versionstring text, + citus_version text, + -- node names must be unique in a given formation UNIQUE (formationid, nodename), -- any nodehost:port can only be a unique node in the system @@ -819,6 +830,30 @@ grant execute on function pgautofailover.set_node_region(text, text, text) to autoctl_node; +-- Deliberately NOT STRICT: version_num/version/versionstring/citus_version +-- are all allowed to be NULL. citus_version legitimately is, whenever +-- Citus isn't installed on that node. This is a plain self-report, called +-- once per Postgres restart by the keeper -- not part of the FSM, and not +-- routed through node_active() since none of this can change without a +-- restart. +CREATE FUNCTION pgautofailover.report_postgres_version + ( + IN node_id bigint, + IN pg_versionnum int default null, + IN pg_version text default null, + IN pg_versionstring text default null, + IN citus_version text default null + ) +RETURNS void LANGUAGE C SECURITY DEFINER +AS 'MODULE_PATHNAME', $$report_postgres_version$$; + +comment on function pgautofailover.report_postgres_version(bigint,int,text,text,text) + is 'reports a node''s Postgres server version and, when installed, Citus extension version'; + +grant execute on function + pgautofailover.report_postgres_version(bigint,int,text,text,text) + to autoctl_node; + create function pgautofailover.synchronous_standby_names ( diff --git a/src/monitor/sql/node_active_protocol.sql b/src/monitor/sql/node_active_protocol.sql index 35b0c9992..355435fda 100644 --- a/src/monitor/sql/node_active_protocol.sql +++ b/src/monitor/sql/node_active_protocol.sql @@ -408,3 +408,35 @@ SELECT pgautofailover.set_node_region('fsm_test', 'unknown_node', 'dc2'); -- empty region: error SELECT pgautofailover.set_node_region('fsm_test', 'node1', ''); + +-- ── test_008: report_postgres_version ──────────────────────────────────────── +-- +-- Postgres/Citus version info is a plain self-report: not part of the FSM, +-- never queried by node_active, and (unlike set_node_region) doesn't error +-- on an unknown node -- a keeper reporting on its own already-known nodeid +-- is a harmless no-op if that row no longer exists, same as +-- ReportAutoFailoverNodeState()'s own report path. + +-- never reported yet: all four columns are NULL +SELECT pg_versionnum, pg_version, pg_versionstring, citus_version + FROM pgautofailover.node WHERE nodeid = :n1; + +SELECT pgautofailover.report_postgres_version( + :n1, 170003, '17.3', 'PostgreSQL 17.3 on x86_64-linux', '12.1'); + +SELECT pg_versionnum, pg_version, pg_versionstring, citus_version + FROM pgautofailover.node WHERE nodeid = :n1; + +-- citus_version omitted (defaults to NULL): a later report legitimately +-- clears a previously-reported citus_version, e.g. Citus was removed +SELECT pgautofailover.report_postgres_version( + :n1, 170003, '17.3', 'PostgreSQL 17.3 on x86_64-linux'); + +SELECT pg_versionnum, pg_version, pg_versionstring, citus_version + FROM pgautofailover.node WHERE nodeid = :n1; + +-- null node_id: error +SELECT pgautofailover.report_postgres_version(NULL, 170003); + +-- unknown node_id: silent no-op, not an error +SELECT pgautofailover.report_postgres_version(-1, 170003); diff --git a/tests/tap/schedules/quick.sch b/tests/tap/schedules/quick.sch index c9d6e7529..f11c55dfc 100644 --- a/tests/tap/schedules/quick.sch +++ b/tests/tap/schedules/quick.sch @@ -2,4 +2,5 @@ basic_operation basic_operation_listen_flag config_get_set +postgres_version_tracking skip_pg_hba diff --git a/tests/tap/specs/postgres_version_tracking.pgaf b/tests/tap/specs/postgres_version_tracking.pgaf new file mode 100644 index 000000000..2bbb60291 --- /dev/null +++ b/tests/tap/specs/postgres_version_tracking.pgaf @@ -0,0 +1,48 @@ +# Test that the monitor's pgautofailover.node row gets the connected node's +# Postgres server version (and, when installed, Citus extension version) +# populated automatically, with no operator action -- the keeper reports +# this once at startup via its own not-running -> running edge detection +# in keeper_update_pg_state(), not through the periodic node_active() report +# (neither piece of information can change without a Postgres restart). +# +# See src/monitor/sql/node_active_protocol.sql's test_008 for direct SQL +# coverage of pgautofailover.report_postgres_version() itself; this spec +# instead exercises the real keeper-side edge-detection and reporting path +# end to end, against a plain (non-Citus) formation. + +cluster { + monitor + formation { + node1 + } +} + +setup { + wait until node1 state is single timeout 60s +} + +teardown { + compose down +} + +step test_001_version_reported_on_startup { + # "single" state (waited on in setup{}) is confirmed via the initial + # registration call, which can land before the keeper's periodic loop + # has run its first keeper_update_pg_state()+keeper_node_active() tick + # -- the actual pairing that fetches and reports version info. Give it + # a moment to complete at least once. + sleep 3s + + sql monitor { SELECT pg_versionnum IS NOT NULL FROM pgautofailover.node WHERE nodename = 'node1'; } + expect { t } + + sql monitor { SELECT pg_version IS NOT NULL FROM pgautofailover.node WHERE nodename = 'node1'; } + expect { t } + + sql monitor { SELECT pg_versionstring LIKE 'PostgreSQL%' FROM pgautofailover.node WHERE nodename = 'node1'; } + expect { t } + + # this formation has no Citus workers; citus_version must stay NULL + sql monitor { SELECT citus_version IS NULL FROM pgautofailover.node WHERE nodename = 'node1'; } + expect { t } +}