Repository navigation
gpstate: fix false "Acting as Primary" report for hot standby mirrors - #1743
jangjang0401 wants to merge 4 commits into
Conversation
leborchuk
left a comment
leborchuk
left a comment
There was a problem hiding this comment.
LGTM. Another approach could be to check recovery.conf for each database instance. However, we cannot do this using SQL. We must connect to each host and run a shell command. After that, we must check if the database is up and the replication slot is active. We should postpone checking recovery. conf until the current approach is no longer sufficient.
Thank you for the review and for sharing the alternative approach. |
|
Thanks for the fix, @jangjang0401. I reproduced the problem and verified the patch on a demo cluster (3 primaries + 3 mirrors, built from this branch in the Rocky 8 build image). With
So healthy hot standbys are no longer flagged, and a mirror that has really been promoted still is. The only consumer of the status string is I pushed one commit on top (a54731c) so this can move forward:
Please squash when merging, since the branch carries a merge from |
|
There is still a false positive when a hot standby loses its WAL receiver connection. With hot_standby=on, the mirror can remain in recovery and accept read-only connections while pg_stat_replication on the primary has no corresponding row. _add_replication_info() then returns None, so this check leaves the status as “Acting as Primary”, even though no promotion occurred.
I've attached the patch with isolation test - isolation2-gpstate.patch. |
|
@Alena0704 You're right, thanks — the absence of a replication row says nothing about promotion. I've disabled auto-merge and pushed two commits:
Run on a demo cluster (Rocky 8, 3 primaries + 3 mirrors),
A mirror really promoted with Could you take another look when you have a moment? Auto-merge stays off until you have. |
|
Thank you so much, @MisterRaindrop , for taking the time to implement the pg_is_in_recovery() fallback, and @Alena0704 , for catching the disconnected case and kindly providing the isolation2 test. I really appreciate your help in moving this PR forward. I've gone through the new commits, and they look good to me. |
|
Here is one behave test PR for the |
|
I think I have found another problem - when the WAL connection is absent, a failed pg_is_in_recovery() query is treated as False, causing gpstate to report “Acting as Primary” without evidence of promotion. I attached the patch with the test. |
I missed your explanation above, so I don't consider it a blocker for merging this PR. |
With hot_standby=on, mirrors accept SQL connections and return PQPING_OK from pg_isready. The previous code unconditionally mapped PQPING_OK + role=mirror to "Acting as Primary", causing gpstate -s to show a spurious warning on every mirror. Fix this in clsSystemState.__buildGpStateData() by cross-checking pg_stat_replication on the primary: if the mirror has an active WAL receiver connection in streaming or catchup state, it is a legitimate hot standby and the status is corrected to "Up". Only fall through to "Acting as Primary" when no such replication connection exists, meaning the segment truly promoted itself to primary. This approach reuses the existing primary connection already established by _add_replication_info(), so no additional database connections are required. _add_replication_info() is updated to return the raw replication state string to make this information available to the caller.
The hot standby check in __buildGpStateData() relies on _add_replication_info() returning the walreceiver's state from pg_stat_replication, and None whenever it has none to give. Cover both: the state is passed through for streaming, catchup and startup, and None comes back when the primary is down, cannot be queried, or does not show exactly one standby connection. Also say in the comment beside the check that a hot standby whose WAL receiver has not reached streaming yet is still reported as "Acting as Primary" until it does.
…AL connection An active WAL receiver connection on the primary proves that a mirror answering PQPING_OK is a hot standby, but its absence proves nothing: a receiver that is starting up, or whose connection has dropped, leaves a mirror that is still in recovery and still accepts read-only connections. Such a mirror was reported as "Acting as Primary" without having been promoted. When there is no streaming or catchup connection, ask the mirror itself with pg_is_in_recovery(). Only a mirror that answers and is out of recovery is still reported as "Acting as Primary"; one that cannot be asked is too, as before. The extra connection is made only for a mirror already reported as "Acting as Primary" without replication, so a healthy cluster pays nothing for it. Reported by Alena Rybakina.
Two cases in the hot_standby schedule. A streaming hot standby must be reported as "Up", and so must one whose WAL connection has been cut while FTS probes are skipped: it is still in recovery and accepting connections, and has not been promoted. primary_conninfo and FTS are restored even when the assertion fails.
aa883eb to
ba34328
Compare
With hot_standby=on, mirrors accept SQL connections and return PQPING_OK
from pg_isready. The previous code unconditionally mapped PQPING_OK +
role=mirror to "Acting as Primary", causing gpstate -s to show a spurious
warning on every mirror.
Fix this in clsSystemState.__buildGpStateData() by cross-checking
pg_stat_replication on the primary: if the mirror has an active WAL
receiver connection in streaming or catchup state, it is a legitimate
hot standby and the status is corrected to "Up". Only fall through to
"Acting as Primary" when no such replication connection exists, meaning
the segment truly promoted itself to primary.
This approach reuses the existing primary connection already established
by _add_replication_info(), so no additional database connections are
required. _add_replication_info() is updated to return the raw
replication state string to make this information available to the
caller.
What does this PR do?
Fixes a false positive in
gpstate -swhere every mirror segment isreported as "Acting as Primary" when
hot_standby=onis enabled.Root cause:
hot_standby=on, mirrors accept read-only SQL connections andpg_isreadyreturnsPQPING_OK(0)instead ofPQPING_MIRROR_READY(64).gpgetstatususingtransition.pyassumedPQPING_OK + role=mirrorcould only mean "the mirror was promoted toprimary", which is no longer true under hot standby.
Fix:
clsSystemState.__buildGpStateData(), cross-check the mirror'sstatus against
pg_stat_replication(already queried on the primaryby
_add_replication_info()).streamingorcatchup), it is a legitimate hot standby → corrected to"Up".promoted itself →
"Acting as Primary"is preserved.Type of Change
Breaking Changes
None. The fix only changes how the status string is derived in
gpstate; cluster configuration, replication behavior, and segmentbehavior are unchanged.
Test Plan
Tested on a 5-VM cluster (1 coordinator + 1 standby coordinator + 3
segment hosts, 6 primary / 6 mirror segments) with
hot_standby=on:Before the fix:
Segment status = Acting as Primarygpstate -sproduced spurious warnings on every mirrorgp_segment_configuration, replication state, andgpstate -mallshowed the cluster as healthy — only the
gpstate -soutput was wrongAfter the fix:
Segment status = UpImpact
Performance:
No additional database connections. The fix consumes data that
_add_replication_info()already collects from the primary; only thereturn value of that function is changed.
User-facing changes:
gpstate -sno longer reports false "Acting as Primary" warnings forhealthy hot-standby mirrors. Genuine promotion is still detected and
reported as before.
Dependencies:
None.
Checklist
Additional Context
An alternative approach considered was to call
pg_is_in_recovery()directly on each mirror inside
_get_segment_status()ingpgetstatususingtransition.py. That approach was rejected because:gpstateinvocation, which is undesirable in unhealthy-cluster scenarios
where
gpstateis run most frequently.primary's view of its replication connections.
The
pg_stat_replicationapproach reuses an existing connection and isauthoritative from the primary's perspective.