Allow wal_log_hints to be changed without restart

Started by Bertrand Drouvot2 months ago2 messageshackers
Beta feature

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.

appliessuccessCI history

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:t248546
psql -h localhost -U postgres

Built from patchset v1 (message #1), August 23, 2026 at 07:10 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 t248546_1 https://github.com/hackorum-dev/postgres.git

In a checkout you already have, add the fork once:

git remote add hackorum https://github.com/hackorum-dev/postgres.git

then, for this patchset and every later one:

git fetch hackorum t248546_1 && git checkout t248546_1

Patchset v1 (message #1) is on t248546_1

Jump to latest
#1Bertrand Drouvot
bertranddrouvot.pg@gmail.com

Hi hackers,

As f19c0ecca introduced online enabling and disabling of data checksums, I
think that it would make sense to do the same for wal_log_hints. Indeed, it
can be used to test how much extra WAL-logging would occur if your database had
data checksums enabled. Since 04bec894a04, data checksums is on by default, but
it could still be set to false due to user choice or after an upgrade.

In that case, I think that it sounds weird to ask for a restart to set wal_log_hints
to test the impact of data checksums that does not require a restart anymore. I
think that the entire flow (test + enable or disable) should be done without a
restart (not just a portion of it).

So, PFA a patch to $SUBJECT.

Basically, it:

- Changes wal_log_hints from PGC_POSTMASTER to PGC_SIGHUP.
- uses the same pattern as full_page_writes as it does not need the complexity
of procsignal barriers that has been used in f19c0ecca. Indeed, it's not a multi
state transition and no all backends must agree simultaneously.

The key concern is that when turning wal_log_hints OFF, no backend should stop
WAL-logging hint bit updates before the parameter change is itself WAL-logged.
Simply propagating the GUC via SIGHUP would leave a window where a backend could
acknowledge the change before the WAL record is written.

To address this, the patch introduces a shared memory flag (XLogCtl->walLogHints)
that serves as the authoritative value read by all backends via XLogHintBitIsNeeded().

Only the checkpointer writes this flag, using the same ordering pattern as
UpdateFullPageWrites():

- When enabling: set the shared flag to true first, then WAL-log the parameter
change. Backends immediately start WAL-logging hints and the extra WAL before the
record is harmless.

- When disabling: WAL-log the parameter change first, then set the
shared flag to false.

As in commit 3b682df3260 (which added the same pattern to UpdateFullPageWrites()),
a critical section is used as extra protection to ensure consistency between the
flag and the WAL record, even though the ordering makes both failure directions
harmless (extra WAL-logging).

As Michael noted in the original thread [1]/messages/by-id/CAB7nPqSXYs4Jg-KJMy8xiM4sTqkgKvHyDrCoojhXzRSKiW57+g@mail.gmail.com, pg_rewind only needs wal_log_hints
to be on "when WAL forked": it only scans WAL from the fork point forward.
Toggling wal_log_hints off and back on before a timeline divergence does not
create a gap, because hint-bit changes from before the fork are irrelevant to
pg_rewind. The pg_rewind check on ControlFile->wal_log_hints (which reflects
the current state at rewind time) remains sufficient.

[1]: /messages/by-id/CAB7nPqSXYs4Jg-KJMy8xiM4sTqkgKvHyDrCoojhXzRSKiW57+g@mail.gmail.com

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

Attachments:

t248546_1
v1-0001-Allow-wal_log_hints-to-be-changed-without-restart.patchtext/x-diff; charset=us-asciiDownload+171-8
#2Tatsuya Kawata
kawatatatsuya0913@gmail.com
In reply to: Bertrand Drouvot (#1)
Re: Allow wal_log_hints to be changed without restart

Hi Bertrand-san,

As f19c0ecca introduced online enabling and disabling of data checksums, I
think that it would make sense to do the same for wal_log_hints.

+1 on the general direction.

The pg_rewind check on ControlFile->wal_log_hints (which reflects
the current state at rewind time) remains sufficient.

I'm not sure this holds. In fact, [1]/messages/by-id/CAB7nPqSXYs4Jg-KJMy8xiM4sTqkgKvHyDrCoojhXzRSKiW57+g@mail.gmail.com seems to say the opposite:

It does not really matter if the node used log_hint_bits set to
false in its latest state (Node to-be-rewinded might have been
restarted after WAL forked).

What pg_rewind actually requires is that the effective value was
continuously on from the divergence point to the end of the target's
WAL. pg_control only keeps the latest value, so an off period after
the divergence goes undetected:

1. Run with wal_log_hints = off (data checksums off).
2. The standby is promoted (divergence).
3. On the old primary, a hint-bit-only page update happens (not WAL-logged).
4. wal_log_hints is set to on via reload (pg_control now says on).
5. The old primary is cleanly shut down and pg_rewind is run.

The check passes, but the page from step 3 never shows up in
extractPageMap() and is not copied from the source. Combined with the
source's CLOG, the leftover hint bit yields incorrect tuple visibility.

I was able to reproduce this: with a transaction held open across the
divergence (insert before the last common checkpoint, commit after the
divergence), the rewound node shows a row that does not exist on the
source, due to a leftover HEAP_XMIN_COMMITTED hint. I can provide this
as a TAP test. Note that 012_wal_log_hints.pl in the patch exercises
exactly this sequence (diverge while off, reload to on, pg_rewind
succeeds) -- the command succeeds, but the result is not consistent.

Strictly speaking, a similar gap can already be created today by
restarting with a different value, but a reload makes it much easier
to hit.

How about making pg_rewind fail if the WAL range it already scans in
extractPageMap() contains an XLOG_PARAMETER_CHANGE record with
wal_log_hints = off? Current pg_control saying on, plus no change
record in the scanned range, guarantees the value was on for the whole
range. This is essentially the tracking that [1]/messages/by-id/CAB7nPqSXYs4Jg-KJMy8xiM4sTqkgKvHyDrCoojhXzRSKiW57+g@mail.gmail.com already suggested on
the pg_rewind side, which does not seem to have been implemented.

[1]: /messages/by-id/CAB7nPqSXYs4Jg-KJMy8xiM4sTqkgKvHyDrCoojhXzRSKiW57+g@mail.gmail.com
/messages/by-id/CAB7nPqSXYs4Jg-KJMy8xiM4sTqkgKvHyDrCoojhXzRSKiW57+g@mail.gmail.com

Regards,
Tatsuya Kawata