[PATCH] Fix TOCTOU races in recovery/t/020_archive_status.pl archive checks
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:t253358psql -h localhost -U postgresBuilt from patchset v1 (message #1), August 23, 2026 at 01:46 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 t253358_1 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 t253358_1 && git checkout t253358_1Patchset v1 (message #1) is on t253358_1
Greetings,
recovery/t/020_archive_status.pl checks archiving progress in two separate
queries: it polls pg_stat_archiver for one condition, then asserts a related
one. For the primary it polls until archived_count is exactly 1, then
separately asserts last_archived_wal equals a specific segment; for the
always-on standby it polls until last_archived_wal equals a segment, then
asserts archived_count is exactly 2.
Both are racy. A checkpoint or crash-recovery segment archived between the
two queries pushes the count or last_archived_wal past the exact value
expected. The exact-equality poll is fragile on its own, too: if the count
jumps past the target, the poll never sees it and times out.
The fix uses monotonic >= instead of exact equality, so the checks tolerate
extra archived segments. WAL segment names sort lexically, so >= is
well defined for last_archived_wal.
--
Bryan Green
EDB: https://www.enterprisedb.com
On Sat, Aug 08, 2026 at 10:47:26PM -0500, Bryan Green wrote:
recovery/t/020_archive_status.pl checks archiving progress in two separate
queries: it polls pg_stat_archiver for one condition, then asserts a related
one. For the primary it polls until archived_count is exactly 1, then
separately asserts last_archived_wal equals a specific segment; for the
always-on standby it polls until last_archived_wal equals a segment, then
asserts archived_count is exactly 2.Both are racy. A checkpoint or crash-recovery segment archived between the
two queries pushes the count or last_archived_wal past the exact value
expected. The exact-equality poll is fragile on its own, too: if the count
jumps past the target, the poll never sees it and times out.The fix uses monotonic >= instead of exact equality, so the checks tolerate
extra archived segments. WAL segment names sort lexically, so >= is
well defined for last_archived_wal.
This test has been around for some time now. Do we have noise in the
buildfarm or in the CI related to it because of extra segments
generated that make these queries fail?
Note: I don't really object to lift these three conditions to comply
more with potential concurrent activity. Just wondering how relevant
this is in practice, particularly when it comes to slow-ish
environments. Or perhaps you have played with this test in a fork?
--
Michael
On 8/10/26 03:25, Michael Paquier wrote:
On Sat, Aug 08, 2026 at 10:47:26PM -0500, Bryan Green wrote:
recovery/t/020_archive_status.pl checks archiving progress in two separate
queries: it polls pg_stat_archiver for one condition, then asserts a related
one. For the primary it polls until archived_count is exactly 1, then
separately asserts last_archived_wal equals a specific segment; for the
always-on standby it polls until last_archived_wal equals a segment, then
asserts archived_count is exactly 2.Both are racy. A checkpoint or crash-recovery segment archived between the
two queries pushes the count or last_archived_wal past the exact value
expected. The exact-equality poll is fragile on its own, too: if the count
jumps past the target, the poll never sees it and times out.The fix uses monotonic >= instead of exact equality, so the checks tolerate
extra archived segments. WAL segment names sort lexically, so >= is
well defined for last_archived_wal.This test has been around for some time now. Do we have noise in the
buildfarm or in the CI related to it because of extra segments
generated that make these queries fail?
No.
Note: I don't really object to lift these three conditions to comply
more with potential concurrent activity. Just wondering how relevant
this is in practice, particularly when it comes to slow-ish
environments. Or perhaps you have played with this test in a fork?
I have been playing in this area with a fork, and looking at it more
closely, this doesn't fix a stock failure. To correct my original note--
a plain CHECKPOINT doesn't force a segment switch, so that wasn't a real
trigger either. The change just trades a little coverage (exact-equality
would catch a spurious extra segment; >= wouldn't) for tolerance of
concurrent archiving that only happens in the fork. I don't have an
upstream case for it and I'm happy to drop it. If you think the added
tolerance is worth keeping as hygiene, I'll leave that to you.
--
Bryan Green
EDB: https://www.enterprisedb.com
On Mon, Aug 10, 2026 at 10:32:05AM -0500, Bryan Green wrote:
I have been playing in this area with a fork, and looking at it more
closely, this doesn't fix a stock failure. To correct my original note--
a plain CHECKPOINT doesn't force a segment switch, so that wasn't a real
trigger either. The change just trades a little coverage (exact-equality
would catch a spurious extra segment; >= wouldn't) for tolerance of
concurrent archiving that only happens in the fork. I don't have an
upstream case for it and I'm happy to drop it. If you think the added
tolerance is worth keeping as hygiene, I'll leave that to you.
This one depends on the community buildfarm for me, and it looks like
I have lost access to the buildfarm database.. Adding Alexander
Lakhin in CC, perhaps he has bumped into a failure on this one.
I miss the highlights in the CI, now I have not seen this test show up
recently there.
@Alexander: Have you noticed some turbulence in the force regarding
the recovery test 020_archive_status.pl?
--
Michael
Hello Michael and Bryan,
11.08.2026 02:46, Michael Paquier wrote:
This one depends on the community buildfarm for me, and it looks like
I have lost access to the buildfarm database.. Adding Alexander
Lakhin in CC, perhaps he has bumped into a failure on this one.I miss the highlights in the CI, now I have not seen this test show up
recently there.@Alexander: Have you noticed some turbulence in the force regarding
the recovery test 020_archive_status.pl?
No, I haven't. Here are all the failures happened on the buildfarm in two
years: [1]https://wiki.postgresql.org/wiki/Known_Buildfarm_Test_Failures, [2]https://wiki.postgresql.org/wiki/Known_Buildfarm_Test_Failures_-_Archive. (I track all current failures with an automatic script, so
no test failure could go unnoticed.)
I can see only one very specific failure of 020_archive_status.pl, that is
not relevant to the subject, I guess: [3]/messages/by-id/f9bebfe6-cee4-ed87-d4e6-29b5ca4be08d@gmail.com.
[1]: https://wiki.postgresql.org/wiki/Known_Buildfarm_Test_Failures
[2]: https://wiki.postgresql.org/wiki/Known_Buildfarm_Test_Failures_-_Archive
[3]: /messages/by-id/f9bebfe6-cee4-ed87-d4e6-29b5ca4be08d@gmail.com
Best regards,
Alexander
On Tue, Aug 11, 2026 at 08:00:00AM +0300, Alexander Lakhin wrote:
No, I haven't. Here are all the failures happened on the buildfarm in two
years: [1], [2]. (I track all current failures with an automatic script, so
no test failure could go unnoticed.)I can see only one very specific failure of 020_archive_status.pl, that is
not relevant to the subject, I guess: [3].
Thanks. I cannot really get excited about changing this script, TBH.
--
Michael