Hot standby accepts connections (giving wrong results) before consistency after crash reset

Hi,

Eric Ridge reported a standby returning wrong query results after a
backend crash. Digging into it, we found that on PG 18 and later a hot
standby can accept read-only connections while replay is still well
behind minRecoveryPoint, and answer them with wrong results and no error
raised!

The trigger is a postmaster crash reset: a child backend exits
abnormally (an OOM kill, a SIGSEGV in an extension, a PANIC, a crashed
worker), restart_after_crash does its job, and the postmaster forks a
replacement startup process.

Commit b53b88109f9 ("Improve error message when standby does accept
connections") made the postmaster maintain reachedConsistency in
addition to the startup process, so that forked backends could pick the
right errdetail. Since the startup process is itself forked from the
postmaster, it now begins life holding whatever value the postmaster
last set. On a crash reset the postmaster re-forks it while its own copy
still says true -- it clears that copy only on receipt of
PMSIGNAL_RECOVERY_STARTED, which the replacement process cannot send
before it exists.

The replacement therefore starts out believing the database is already
consistent, and CheckRecoveryConsistency() skips the minRecoveryPoint
comparison entirely. Hot standby is announced at redo start, while redo
restarts from the redo pointer in the control file, which can be far
behind minRecoveryPoint. Connections admitted in that interval read heap
pages that the previous startup process flushed ahead of this pass's
replay position!

The interval lasts until replay reaches minRecoveryPoint, so its length
is set by whatever throttles redo rather than by the byte distance
involved. On a caught-up standby it is very short, which is likely why
this went unnoticed since April, 2025. On a standby configured with
recovery_min_apply_delay it is much longer: recoveryApplyDelay() returns
early while reachedConsistency is false, so with the flag wrongly true
the delay is honoured from the first commit record onward and redo parks
near the start of the range.

Two further notes that may save someone time reproducing it:

- Successive crash resets alternate. A startup process that skips the
branch never sends PMSIGNAL_RECOVERY_CONSISTENT, so the postmaster's
copy stays false and the next reset forks a process holding the correct
value. That pass reaches consistency properly, which sets the
postmaster's copy back to true and re-arms the problem for the reset
after it. Six consecutive kills in one server lifetime give bad, good,
bad, good, bad, good. It is therefore not a one-off following startup:
roughly every other crash reset is affected, for as long as the server
runs.

- EXEC_BACKEND builds are unaffected, since reachedConsistency is not
carried in BackendParameters. (That also means b53b88109f9's own
errdetail selection does not work there, which is a separate cosmetic
issue I have not addressed here.)

I bisected this to b53b88109f9 and confirmed it by building its parent
and the commit itself: the parent is clean, the commit reproduces. 17.9
is unaffected -- its postmaster never references reachedConsistency at
all -- and 18.4, 19beta2 and master all reproduce.

Attached are two patches:

0001 adds a TAP test. It fails on master and on back branches down to
18, so a reviewer can confirm the problem before applying the fix.
The test uses recovery_min_apply_delay to keep the check from
depending on timing: recoveryApplyDelay() ignores the delay until
consistency is reached, so a correct standby replays to
minRecoveryPoint at full speed while an affected one stops well
short of it.

0002 clears reachedConsistency in InitWalRecovery(), so that a startup
process never depends on the value it inherited. The postmaster's
own copy is left alone, since forked backends still read it for
the errdetail, and it converges once the new startup process sends
PMSIGNAL_RECOVERY_STARTED and later
PMSIGNAL_RECOVERY_CONSISTENT.

I put the reset in the startup process rather than having the postmaster
clear its copy before forking, so that the invariant lives with the
process that owns it and no future fork path has to remember to clear it
first. A tidier alternative would be to give the postmaster its own
flag and return reachedConsistency to being startup-process-private, as
it effectively was through 17.x, but that seemed too invasive for
something that wants back-patching to 18. I am happy to write it that
way for master if people prefer.

make check-world is clean with the fix, built with --enable-cassert and
--enable-injection-points.

I also have a couple of small shell scripts I used while investigating:
one measures how long a standby stays open below minRecoveryPoint under
different configurations, and one demonstrates the alternating
affected/unaffected behaviour across successive crash resets. They are
not part of the patch so I have not included them here, but I am happy to
share them if anyone wants to cross-check the behaviour independently.

Thanks,
Nikhil

---
Nikhil Sontakke
PlanetScale Postgres Core Team