Lazy allocation of pages required for verifying FPI consistency
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:t47222psql -h localhost -U postgresBuilt from patchset v1 (message #1), July 28, 2026 at 01:22 AM.
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 t47222_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 t47222_1 && git checkout t47222_1Patchset v1 (message #1) is on t47222_1
Hi,
Postgres verifies consistency of FPI from WAL record with the replayed
page during recovery in verifyBackupPageConsistency() when either
wal_consistency_checking for the resource manager is enabled or a WAL
record with XLR_CHECK_CONSISTENCY flag is inserted. While doing so, it
uses two intermediate pages primary_image_masked (FPI from WAL record)
and replay_image_masked (replayed page) which are dynamically
allocated (palloc'd) before the recovery starts, however, they're not
used unless verifyBackupPageConsistency() is called. And these
variables are palloc'd here for getting MAXALIGNed memory as opposed
to static char arrays. Since verifyBackupPageConsistency() gets called
conditionally only when the WAL record has the XLR_CHECK_CONSISTENCY
flag set, it's a waste of memory for these two page variables.
I propose to statically allocate these two pages using PGAlignedBlock
structure lazily in verifyBackupPageConsistency() to not waste dynamic
memory worth 2*BLCKSZ bytes. I'm attaching a small patch herewith.
Thoughts?
--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
At Mon, 9 Jan 2023 20:00:00 +0530, Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> wrote in
I propose to statically allocate these two pages using PGAlignedBlock
structure lazily in verifyBackupPageConsistency() to not waste dynamic
memory worth 2*BLCKSZ bytes. I'm attaching a small patch herewith.Thoughts?
IMHO, it's a bit scaring to me to push down the execution stack by
that large size. I tend to choose the (current) possible memory
wasting only on startup process than digging stack deeply.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
On Thu, Jan 12, 2023 at 4:29 PM Kyotaro Horiguchi
<horikyota.ntt@gmail.com> wrote:
At Mon, 9 Jan 2023 20:00:00 +0530, Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> wrote in
I propose to statically allocate these two pages using PGAlignedBlock
structure lazily in verifyBackupPageConsistency() to not waste dynamic
memory worth 2*BLCKSZ bytes. I'm attaching a small patch herewith.Thoughts?
IMHO, it's a bit scaring to me to push down the execution stack by
that large size. I tend to choose the (current) possible memory
wasting only on startup process than digging stack deeply.
+1
On Thu, Jan 12, 2023 at 1:59 PM Kyotaro Horiguchi
<horikyota.ntt@gmail.com> wrote:
At Mon, 9 Jan 2023 20:00:00 +0530, Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> wrote in
I propose to statically allocate these two pages using PGAlignedBlock
structure lazily in verifyBackupPageConsistency() to not waste dynamic
memory worth 2*BLCKSZ bytes. I'm attaching a small patch herewith.Thoughts?
IMHO, it's a bit scaring to me to push down the execution stack by
that large size. I tend to choose the (current) possible memory
wasting only on startup process than digging stack deeply.
On the contrary, PGAlignedBlock is being used elsewhere in the code;
some of them are hot paths. verifyBackupPageConsistency() is not
something that gets called always i.e. WAL consistency checks are done
conditionally - when either one enables wal_consistency_checking for
the rmgr or the WAL record is flagged with
XLR_CHECK_CONSISTENCY (core doesn't do, it's an external module, if
any, do that).
I really don't see much of a problem in allocating them statically and
pushing closer to where they're being used. If this really concerns,
at the least, the dynamic allocation needs to be pushed to
verifyBackupPageConsistency() IMO with if (first_time) { allocate two
blocks with palloc} and use them. This at least saves some memory on
the heap for most of the servers out there.
--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
On Thu, Jan 12, 2023 at 04:37:38PM +0800, Julien Rouhaud wrote:
On Thu, Jan 12, 2023 at 4:29 PM Kyotaro Horiguchi
<horikyota.ntt@gmail.com> wrote:IMHO, it's a bit scaring to me to push down the execution stack by
that large size. I tend to choose the (current) possible memory
wasting only on startup process than digging stack deeply.+1
Indeed. I agree to leave that be.
--
Michael
At Thu, 12 Jan 2023 15:02:25 +0530, Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> wrote in
On the contrary, PGAlignedBlock is being used elsewhere in the code;
I noticed it and had the same feeling, and thought that they don't
justify to do the same at other places.
some of them are hot paths. verifyBackupPageConsistency() is not
something that gets called always i.e. WAL consistency checks are done
conditionally - when either one enables wal_consistency_checking for
the rmgr or the WAL record is flagged with
XLR_CHECK_CONSISTENCY (core doesn't do, it's an external module, if
any, do that).
Right. So we could allocate them at the first use as below, but...
I really don't see much of a problem in allocating them statically and
pushing closer to where they're being used. If this really concerns,
at the least, the dynamic allocation needs to be pushed to
verifyBackupPageConsistency() IMO with if (first_time) { allocate two
blocks with palloc} and use them. This at least saves some memory on
the heap for most of the servers out there.
Yeah, we could do that. But as I mentioned before, that happens only
on startup thus it can be said that that's not worth bothering. On
the other hand I don't think it's great to waste 16kB * max_backends
memory especially when it is clearly recognized and easily avoidable.
I guess the reason for the code is more or less that.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
On Mon, Jan 16, 2023 at 10:52:43AM +0900, Kyotaro Horiguchi wrote:
Yeah, we could do that. But as I mentioned before, that happens only
on startup thus it can be said that that's not worth bothering. On
the other hand I don't think it's great to waste 16kB * max_backends
memory especially when it is clearly recognized and easily avoidable.
Memory's cheap, but basically nobody would use these except
developers..
I guess the reason for the code is more or less that.
The original discussion spreads across these threads:
/messages/by-id/CAB7nPqR0jzhF=U4AXLm+cmaE4J-HkUzbaRXtg+6ieERTqr=pcg@mail.gmail.com
/messages/by-id/CAGz5QC+_CNcDJkkmDyPg2zJ_R8AtEg1KyYADbU6B673RaTySAg@mail.gmail.com
There was a specific point about using static buffers from me, though
these would not have been aligned as of the lack of PGAlignedBlock
back in 2017 which is why palloc() was used. That should be around
here:
/messages/by-id/CAB7nPqR=OcojLCP=1Ho6Zo312CKzUZU8d4aJO+VvpUYV-waU_Q@mail.gmail.com
--
Michael