Hot standby accepts connections (giving wrong results) before consistency after crash reset
Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.
You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:
docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t253345psql -h localhost -U postgresBuilt from patchset v4 (message #4), August 23, 2026 at 01:52 PM.
Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:
git clone --branch t253345_4 https://github.com/hackorum-dev/postgres.gitIn a checkout you already have, add the fork once:
git remote add hackorum https://github.com/hackorum-dev/postgres.gitthen, for this patchset and every later one:
git fetch hackorum t253345_4 && git checkout t253345_4Patchset v4 (message #4) is on t253345_4
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
Attachments:
t253345_10001-Add-a-TAP-test-for-hot-standby-consistency-after-a-c.patchapplication/octet-stream; name=0001-Add-a-TAP-test-for-hot-standby-consistency-after-a-c.patchDownload+215-1
0002-Fix-hot-standby-accepting-connections-too-early-afte.patchapplication/octet-stream; name=0002-Fix-hot-standby-accepting-connections-too-early-afte.patchDownload+26-1
On Fri, Aug 7, 2026 at 6:52 PM Nikhil Sontakke <nikhil@planetscale.com> wrote:
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.
Thanks for the report, analysis, and patches!
Your analysis looks correct to me, and 0002 seems like the right minimal fix.
I've pushed 0002, with a slightly shorter source comment. The original comment
was a bit too detailed for an in-code comment; I think those details are better
covered by the commit message.
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.
Agreed. For the backpatchable fix, resetting reachedConsistency in the
startup process seems simple and sufficient.
In the longer term, it may be worth separating the postmaster's state
from the startup process's state on master, since they use this
information for slightly different purposes. But I don't think that is
necessary for this bug fix.
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.
I have not committed 0001 for now.
The test does cover the problem, but it seems somewhat expensive for
what it checks, since it creates a 20,000-row table and updates the
whole table several times to create the required gap. I'd like to
discuss whether we really want the test in this form, or whether the
same coverage can be achieved more cheaply or simply.
Regards,
--
Fujii Masao
Hi Fujii-san,
Thanks for pushing the fix. I agree that keeping the source comment concise
is
better, and I'm happy to look into refining the TAP test to be more
lightweight
while still providing the necessary coverage. Let me know if that's what we
want
to do.
Regards,
Nikhil
On Fri, Aug 7, 2026 at 8:43 PM Fujii Masao <masao.fujii@gmail.com> wrote:
Show quoted text
On Fri, Aug 7, 2026 at 6:52 PM Nikhil Sontakke <nikhil@planetscale.com>
wrote: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.Thanks for the report, analysis, and patches!
Your analysis looks correct to me, and 0002 seems like the right minimal
fix.I've pushed 0002, with a slightly shorter source comment. The original
comment
was a bit too detailed for an in-code comment; I think those details are
better
covered by the commit message.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.Agreed. For the backpatchable fix, resetting reachedConsistency in the
startup process seems simple and sufficient.In the longer term, it may be worth separating the postmaster's state
from the startup process's state on master, since they use this
information for slightly different purposes. But I don't think that is
necessary for this bug fix.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.I have not committed 0001 for now.
The test does cover the problem, but it seems somewhat expensive for
what it checks, since it creates a 20,000-row table and updates the
whole table several times to create the required gap. I'd like to
discuss whether we really want the test in this form, or whether the
same coverage can be achieved more cheaply or simply.Regards,
--
Fujii Masao
Hi Fujii-san,
You are right that the test was expensive for what it checks. I have
reworked it: rather than adding a new file, v2 extends the existing
016_min_consistency.pl tap test.
The new test apparatus turns out to be unnecessary, because
016_min_consistency.pl already builds it. It runs with the same
shared_buffers = 128kB, its table is large enough at fillfactor 10 to
keep the startup process evicting, and at the point where it takes its
restart point no restartpoint has run yet, so the redo pointer in the
control file is still the one taken by the base backup.
So v2 adds a phase to that test, between the last wait_for_catchup and
the CHECKPOINT that creates the restart point. It kills a backend to
force the crash reset, then checks that replay gets back up to
minRecoveryPoint before a connection is accepted, and that the first
connection admitted afterwards sees the table as it should be.
Numbers on my machine: 10 runs with the fix all pass, 10 runs with it
reverted all fail, and the recovery positions are byte-identical across
every run, so it is not timing-sensitive. The added runtime is two to
three seconds. 016's own assertion passes in both directions, and
make check in src/test/recovery is green.
Please let me know if this works for you.
Thanks,
Nikhil
---
Nikhil Sontakke
PlanetScale
On Fri, Aug 7, 2026 at 9:21 PM Nikhil Sontakke <nikhil@planetscale.com>
wrote:
Show quoted text
Hi Fujii-san,
Thanks for pushing the fix. I agree that keeping the source comment
concise is
better, and I'm happy to look into refining the TAP test to be more
lightweight
while still providing the necessary coverage. Let me know if that's what
we want
to do.Regards,
NikhilOn Fri, Aug 7, 2026 at 8:43 PM Fujii Masao <masao.fujii@gmail.com> wrote:
On Fri, Aug 7, 2026 at 6:52 PM Nikhil Sontakke <nikhil@planetscale.com>
wrote:0002 clears reachedConsistency in InitWalRecovery(), so that a startup
process never depends on the value it inherited. Thepostmaster's
own copy is left alone, since forked backends still read it for
the errdetail, and it converges once the new startup processsends
PMSIGNAL_RECOVERY_STARTED and later
PMSIGNAL_RECOVERY_CONSISTENT.Thanks for the report, analysis, and patches!
Your analysis looks correct to me, and 0002 seems like the right minimal
fix.I've pushed 0002, with a slightly shorter source comment. The original
comment
was a bit too detailed for an in-code comment; I think those details are
better
covered by the commit message.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.Agreed. For the backpatchable fix, resetting reachedConsistency in the
startup process seems simple and sufficient.In the longer term, it may be worth separating the postmaster's state
from the startup process's state on master, since they use this
information for slightly different purposes. But I don't think that is
necessary for this bug fix.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 thefix.
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.I have not committed 0001 for now.
The test does cover the problem, but it seems somewhat expensive for
what it checks, since it creates a 20,000-row table and updates the
whole table several times to create the required gap. I'd like to
discuss whether we really want the test in this form, or whether the
same coverage can be achieved more cheaply or simply.Regards,
--
Fujii Masao