pg.ddx.io pgsql-hackers@postgresql.org mailing list archivehelp / color / mirror / Atom feed
[PATCH v1] Allow wal_log_hints to be changed without restart 3+ messages / 2 participants [nested] [flat]
* [PATCH v1] Allow wal_log_hints to be changed without restart @ 2026-06-13 07:20 Bertrand Drouvot <bertranddrouvot.pg@gmail.com> 0 siblings, 0 replies; 3+ messages in thread From: Bertrand Drouvot @ 2026-06-13 07:20 UTC (permalink / raw) Change wal_log_hints from PGC_POSTMASTER to PGC_SIGHUP, allowing it to be changed without restarting the server. As f19c0ecca introduced online enabling and disabling of data checksums, it makes sense to do the same for wal_log_hints (as it can be used to test how much extra WAL-logging would occur if your database had data checksums enabled). This commit: - 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, we introduce 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). Author: Bertrand Drouvot <bertranddrouvot.pg@gmail.com> Reviewed-by: Discussion: --- doc/src/sgml/config.sgml | 4 +- src/backend/access/transam/xlog.c | 82 +++++++++++++++++++++++ src/backend/postmaster/checkpointer.c | 6 ++ src/backend/utils/misc/guc_parameters.dat | 2 +- src/bin/pg_rewind/t/012_wal_log_hints.pl | 77 +++++++++++++++++++++ src/include/access/xlog.h | 3 +- 6 files changed, 171 insertions(+), 3 deletions(-) 5.0% doc/src/sgml/ 44.8% src/backend/access/transam/ 3.3% src/backend/utils/misc/ 41.4% src/bin/pg_rewind/t/ 5.3% src/ diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index fa566c9e553..ffa6235a5ba 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -3646,7 +3646,9 @@ include_dir 'conf.d' </para> <para> - This parameter can only be set at server start. The default value is <literal>off</literal>. + This parameter can only be set in the <filename>postgresql.conf</filename> + file or on the server command line. + The default value is <literal>off</literal>. </para> </listitem> </varlistentry> diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index 6c2304fef33..f0ba2291433 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -559,6 +559,16 @@ typedef struct XLogCtlData /* last data_checksum_version we've seen */ uint32 data_checksum_version; + /* + * walLogHints is the authoritative value used by all backends to + * determine whether to WAL-log hint bit updates. This shared value, + * instead of the process-local wal_log_hints, is required because, when + * wal_log_hints is changed by SIGHUP, we must ensure proper ordering + * between the WAL parameter change record and the actual behavior change. + * Checkpointer updates it after SIGHUP. + */ + bool walLogHints; + slock_t info_lck; /* locks shared variables shown above */ } XLogCtlData; @@ -4678,6 +4688,23 @@ DataChecksumsNeedWrite(void) LocalDataChecksumState == PG_DATA_CHECKSUM_INPROGRESS_OFF); } +/* + * XLogHintBitIsNeeded + * Returns whether hint bit must be written or not + * + * Returns true if wal_log_hints is enabled in shared memory, or if data + * checksums require writes. The shared memory value is used (rather than + * the process-local wal_log_hints) to ensure all backends observe the change + * atomically with respect to the WAL record that logs the state transition. + */ +bool +XLogHintBitIsNeeded(void) +{ + volatile XLogCtlData *xlogctl = XLogCtl; + + return (xlogctl->walLogHints || DataChecksumsNeedWrite()); +} + bool DataChecksumsOff(void) @@ -5429,6 +5456,9 @@ XLOGShmemInit(void *arg) XLogCtl->data_checksum_version = ControlFile->data_checksum_version; SetLocalDataChecksumState(XLogCtl->data_checksum_version); + /* Use wal_log_hints from control file */ + XLogCtl->walLogHints = ControlFile->wal_log_hints; + SpinLockInit(&XLogCtl->Insert.insertpos_lck); SpinLockInit(&XLogCtl->info_lck); pg_atomic_init_u64(&XLogCtl->logInsertResult, InvalidXLogRecPtr); @@ -6555,6 +6585,9 @@ StartupXLOG(void) Insert->fullPageWrites = lastFullPageWrites; UpdateFullPageWrites(); + /* Update wal_log_hints in shared memory */ + UpdateWalLogHints(); + /* * Emit checkpoint or end-of-recovery record in XLOG, if required. */ @@ -8812,6 +8845,55 @@ UpdateFullPageWrites(void) END_CRIT_SECTION(); } +/* + * Update wal_log_hints in shared memory, and write an + * XLOG_PARAMETER_CHANGE record if necessary. + * + * This follows the same ordering pattern as UpdateFullPageWrites(): + * when setting to true, update shared memory first (so backends start + * WAL-logging hints before the WAL record), and when setting to false, + * write the WAL record first (so the change is logged before backends + * stop WAL-logging hints). + */ +void +UpdateWalLogHints(void) +{ + bool recoveryInProgress; + + if (wal_log_hints == XLogCtl->walLogHints) + return; + + /* + * Perform this outside critical section so that the WAL insert + * initialization done by RecoveryInProgress() doesn't trigger an + * assertion failure. + */ + recoveryInProgress = RecoveryInProgress(); + + START_CRIT_SECTION(); + + /* + * It's always safe to WAL-log hint bits, even when not strictly required, + * but not the other round. So if we're setting wal_log_hints to true, + * first set it true and then write the WAL record. If we're setting it to + * false, first write the WAL record and then set the global flag. + */ + if (wal_log_hints) + XLogCtl->walLogHints = true; + + /* + * XLogReportParameters will WAL-log and update the control file. During + * recovery, we can't write WAL so just update the shared flag. + */ + if (!recoveryInProgress) + XLogReportParameters(); + + if (!wal_log_hints) + XLogCtl->walLogHints = false; + + END_CRIT_SECTION(); +} + /* * XLOG resource manager's routines * diff --git a/src/backend/postmaster/checkpointer.c b/src/backend/postmaster/checkpointer.c index 087120db090..463a24edf55 100644 --- a/src/backend/postmaster/checkpointer.c +++ b/src/backend/postmaster/checkpointer.c @@ -1510,6 +1510,12 @@ UpdateSharedMemoryConfig(void) */ UpdateFullPageWrites(); + /* + * If wal_log_hints has been changed by SIGHUP, we update pg_control and + * write an XLOG_PARAMETER_CHANGE record. + */ + UpdateWalLogHints(); + elog(DEBUG2, "checkpointer updated shared memory configuration values"); } diff --git a/src/backend/utils/misc/guc_parameters.dat b/src/backend/utils/misc/guc_parameters.dat index afaa058b046..1a39fa09ea1 100644 --- a/src/backend/utils/misc/guc_parameters.dat +++ b/src/backend/utils/misc/guc_parameters.dat @@ -3502,7 +3502,7 @@ options => 'wal_level_options', }, -{ name => 'wal_log_hints', type => 'bool', context => 'PGC_POSTMASTER', group => 'WAL_SETTINGS', +{ name => 'wal_log_hints', type => 'bool', context => 'PGC_SIGHUP', group => 'WAL_SETTINGS', short_desc => 'Writes full pages to WAL when first modified after a checkpoint, even for a non-critical modification.', variable => 'wal_log_hints', boot_val => 'false', diff --git a/src/bin/pg_rewind/t/012_wal_log_hints.pl b/src/bin/pg_rewind/t/012_wal_log_hints.pl new file mode 100644 index 00000000000..9eb9c48db7c --- /dev/null +++ b/src/bin/pg_rewind/t/012_wal_log_hints.pl @@ -0,0 +1,77 @@ + +# Copyright (c) 2021-2026, PostgreSQL Global Development Group + +# +# Test pg_rewind interaction with wal_log_hints: +# - Error out when wal_log_hints=off and no data checksums +# - Succeeds after reload to on +# +use strict; +use warnings FATAL => 'all'; +use PostgreSQL::Test::Cluster; +use PostgreSQL::Test::Utils; +use Test::More; + +# Initialize primary without data checksums and with wal_log_hints=off +my $node_primary = PostgreSQL::Test::Cluster->new('primary'); +$node_primary->init(allows_streaming => 1, no_data_checksums => 1); +$node_primary->append_conf( + 'postgresql.conf', qq{ +wal_log_hints = off +wal_level = replica +wal_keep_size = 64MB +autovacuum = off +}); +$node_primary->start; + +$node_primary->safe_psql('postgres', + "CREATE TABLE t(id int); INSERT INTO t SELECT generate_series(1,100)"); +$node_primary->safe_psql('postgres', "CHECKPOINT"); + +# Create standby, diverge +my $node_standby = PostgreSQL::Test::Cluster->new('standby'); +$node_primary->backup('my_backup'); +$node_standby->init_from_backup($node_primary, 'my_backup', + has_streaming => 1); +$node_standby->start; +$node_primary->wait_for_catchup($node_standby); + +$node_standby->promote; +$node_standby->safe_psql('postgres', "INSERT INTO t VALUES (999)"); +$node_primary->safe_psql('postgres', "INSERT INTO t VALUES (888)"); +$node_primary->safe_psql('postgres', "CHECKPOINT"); +$node_primary->stop; + +# pg_rewind must refuse: wal_log_hints=off +command_fails_like( + [ + 'pg_rewind', + '--target-pgdata' => $node_primary->data_dir, + '--source-server' => $node_standby->connstr('postgres'), + '--no-sync', + ], + qr/target server needs to use either data checksums or "wal_log_hints = on"/, + 'pg_rewind refuses with wal_log_hints=off and no data checksums'); + +# Restart primary and enable wal_log_hints via reload +$node_primary->start; +$node_primary->append_conf('postgresql.conf', 'wal_log_hints = on'); +$node_primary->reload; + +my $result = $node_primary->safe_psql('postgres', "SHOW wal_log_hints"); +is($result, 'on', 'wal_log_hints changed to on via reload'); + +$node_primary->stop; + +# Same pg_rewind now succeeds +command_ok( + [ + 'pg_rewind', + '--target-pgdata' => $node_primary->data_dir, + '--source-server' => $node_standby->connstr('postgres'), + '--no-sync', + ], + 'pg_rewind succeeds after enabling wal_log_hints via reload'); + +$node_standby->stop; +done_testing(); diff --git a/src/include/access/xlog.h b/src/include/access/xlog.h index 4dd98624204..8016eaadb53 100644 --- a/src/include/access/xlog.h +++ b/src/include/access/xlog.h @@ -120,7 +120,7 @@ extern PGDLLIMPORT bool XLogLogicalInfo; * of the bits make it to disk, but the checksum wouldn't match. Also WAL-log * them if forced by wal_log_hints=on. */ -#define XLogHintBitIsNeeded() (wal_log_hints || DataChecksumsNeedWrite()) +extern bool XLogHintBitIsNeeded(void); /* Do we need to WAL-log information required only for Hot Standby and logical replication? */ #define XLogStandbyInfoActive() (wal_level >= WAL_LEVEL_REPLICA) @@ -274,6 +274,7 @@ extern void XLogPutNextOid(Oid nextOid); extern XLogRecPtr XLogRestorePoint(const char *rpName); extern XLogRecPtr XLogAssignLSN(void); extern void UpdateFullPageWrites(void); +extern void UpdateWalLogHints(void); extern void GetFullPageWriteInfo(XLogRecPtr *RedoRecPtr_p, bool *doPageWrites_p); extern XLogRecPtr GetRedoRecPtr(void); extern XLogRecPtr GetInsertRecPtr(void); -- 2.34.1 --9ItUckmLlnvfmpcW-- ^ permalink raw reply [nested|flat] 3+ messages in thread
* Allow wal_log_hints to be changed without restart @ 2026-06-15 13:40 Bertrand Drouvot <bertranddrouvot.pg@gmail.com> 0 siblings, 1 reply; 3+ messages in thread From: Bertrand Drouvot @ 2026-06-15 13:40 UTC (permalink / raw) To: pgsql-hackers@lists.postgresql.org 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], 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] https://postgr.es/m/CAB7nPqSXYs4Jg-KJMy8xiM4sTqkgKvHyDrCoojhXzRSKiW57+g@mail.gmail.com Regards, -- Bertrand Drouvot PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com Attachments: [text/x-diff] v1-0001-Allow-wal_log_hints-to-be-changed-without-restart.patch (11.6K, ../../ajAAwSFy0WVMroyk@bdtpg/2-v1-0001-Allow-wal_log_hints-to-be-changed-without-restart.patch) download | inline diff: From e1729c994aed9baa87e63875f6567b34f6d06b98 Mon Sep 17 00:00:00 2001 From: Bertrand Drouvot <bertranddrouvot.pg@gmail.com> Date: Sat, 13 Jun 2026 07:20:08 +0000 Subject: [PATCH v1] Allow wal_log_hints to be changed without restart Change wal_log_hints from PGC_POSTMASTER to PGC_SIGHUP, allowing it to be changed without restarting the server. As f19c0ecca introduced online enabling and disabling of data checksums, it makes sense to do the same for wal_log_hints (as it can be used to test how much extra WAL-logging would occur if your database had data checksums enabled). This commit: - 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, we introduce 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). Author: Bertrand Drouvot <bertranddrouvot.pg@gmail.com> Reviewed-by: Discussion: --- doc/src/sgml/config.sgml | 4 +- src/backend/access/transam/xlog.c | 82 +++++++++++++++++++++++ src/backend/postmaster/checkpointer.c | 6 ++ src/backend/utils/misc/guc_parameters.dat | 2 +- src/bin/pg_rewind/t/012_wal_log_hints.pl | 77 +++++++++++++++++++++ src/include/access/xlog.h | 3 +- 6 files changed, 171 insertions(+), 3 deletions(-) 5.0% doc/src/sgml/ 44.8% src/backend/access/transam/ 3.3% src/backend/utils/misc/ 41.4% src/bin/pg_rewind/t/ 5.3% src/ diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index fa566c9e553..ffa6235a5ba 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -3646,7 +3646,9 @@ include_dir 'conf.d' </para> <para> - This parameter can only be set at server start. The default value is <literal>off</literal>. + This parameter can only be set in the <filename>postgresql.conf</filename> + file or on the server command line. + The default value is <literal>off</literal>. </para> </listitem> </varlistentry> diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index 6c2304fef33..f0ba2291433 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -559,6 +559,16 @@ typedef struct XLogCtlData /* last data_checksum_version we've seen */ uint32 data_checksum_version; + /* + * walLogHints is the authoritative value used by all backends to + * determine whether to WAL-log hint bit updates. This shared value, + * instead of the process-local wal_log_hints, is required because, when + * wal_log_hints is changed by SIGHUP, we must ensure proper ordering + * between the WAL parameter change record and the actual behavior change. + * Checkpointer updates it after SIGHUP. + */ + bool walLogHints; + slock_t info_lck; /* locks shared variables shown above */ } XLogCtlData; @@ -4678,6 +4688,23 @@ DataChecksumsNeedWrite(void) LocalDataChecksumState == PG_DATA_CHECKSUM_INPROGRESS_OFF); } +/* + * XLogHintBitIsNeeded + * Returns whether hint bit must be written or not + * + * Returns true if wal_log_hints is enabled in shared memory, or if data + * checksums require writes. The shared memory value is used (rather than + * the process-local wal_log_hints) to ensure all backends observe the change + * atomically with respect to the WAL record that logs the state transition. + */ +bool +XLogHintBitIsNeeded(void) +{ + volatile XLogCtlData *xlogctl = XLogCtl; + + return (xlogctl->walLogHints || DataChecksumsNeedWrite()); +} + bool DataChecksumsOff(void) @@ -5429,6 +5456,9 @@ XLOGShmemInit(void *arg) XLogCtl->data_checksum_version = ControlFile->data_checksum_version; SetLocalDataChecksumState(XLogCtl->data_checksum_version); + /* Use wal_log_hints from control file */ + XLogCtl->walLogHints = ControlFile->wal_log_hints; + SpinLockInit(&XLogCtl->Insert.insertpos_lck); SpinLockInit(&XLogCtl->info_lck); pg_atomic_init_u64(&XLogCtl->logInsertResult, InvalidXLogRecPtr); @@ -6555,6 +6585,9 @@ StartupXLOG(void) Insert->fullPageWrites = lastFullPageWrites; UpdateFullPageWrites(); + /* Update wal_log_hints in shared memory */ + UpdateWalLogHints(); + /* * Emit checkpoint or end-of-recovery record in XLOG, if required. */ @@ -8812,6 +8845,55 @@ UpdateFullPageWrites(void) END_CRIT_SECTION(); } +/* + * Update wal_log_hints in shared memory, and write an + * XLOG_PARAMETER_CHANGE record if necessary. + * + * This follows the same ordering pattern as UpdateFullPageWrites(): + * when setting to true, update shared memory first (so backends start + * WAL-logging hints before the WAL record), and when setting to false, + * write the WAL record first (so the change is logged before backends + * stop WAL-logging hints). + */ +void +UpdateWalLogHints(void) +{ + bool recoveryInProgress; + + if (wal_log_hints == XLogCtl->walLogHints) + return; + + /* + * Perform this outside critical section so that the WAL insert + * initialization done by RecoveryInProgress() doesn't trigger an + * assertion failure. + */ + recoveryInProgress = RecoveryInProgress(); + + START_CRIT_SECTION(); + + /* + * It's always safe to WAL-log hint bits, even when not strictly required, + * but not the other round. So if we're setting wal_log_hints to true, + * first set it true and then write the WAL record. If we're setting it to + * false, first write the WAL record and then set the global flag. + */ + if (wal_log_hints) + XLogCtl->walLogHints = true; + + /* + * XLogReportParameters will WAL-log and update the control file. During + * recovery, we can't write WAL so just update the shared flag. + */ + if (!recoveryInProgress) + XLogReportParameters(); + + if (!wal_log_hints) + XLogCtl->walLogHints = false; + + END_CRIT_SECTION(); +} + /* * XLOG resource manager's routines * diff --git a/src/backend/postmaster/checkpointer.c b/src/backend/postmaster/checkpointer.c index 087120db090..463a24edf55 100644 --- a/src/backend/postmaster/checkpointer.c +++ b/src/backend/postmaster/checkpointer.c @@ -1510,6 +1510,12 @@ UpdateSharedMemoryConfig(void) */ UpdateFullPageWrites(); + /* + * If wal_log_hints has been changed by SIGHUP, we update pg_control and + * write an XLOG_PARAMETER_CHANGE record. + */ + UpdateWalLogHints(); + elog(DEBUG2, "checkpointer updated shared memory configuration values"); } diff --git a/src/backend/utils/misc/guc_parameters.dat b/src/backend/utils/misc/guc_parameters.dat index afaa058b046..1a39fa09ea1 100644 --- a/src/backend/utils/misc/guc_parameters.dat +++ b/src/backend/utils/misc/guc_parameters.dat @@ -3502,7 +3502,7 @@ options => 'wal_level_options', }, -{ name => 'wal_log_hints', type => 'bool', context => 'PGC_POSTMASTER', group => 'WAL_SETTINGS', +{ name => 'wal_log_hints', type => 'bool', context => 'PGC_SIGHUP', group => 'WAL_SETTINGS', short_desc => 'Writes full pages to WAL when first modified after a checkpoint, even for a non-critical modification.', variable => 'wal_log_hints', boot_val => 'false', diff --git a/src/bin/pg_rewind/t/012_wal_log_hints.pl b/src/bin/pg_rewind/t/012_wal_log_hints.pl new file mode 100644 index 00000000000..9eb9c48db7c --- /dev/null +++ b/src/bin/pg_rewind/t/012_wal_log_hints.pl @@ -0,0 +1,77 @@ + +# Copyright (c) 2021-2026, PostgreSQL Global Development Group + +# +# Test pg_rewind interaction with wal_log_hints: +# - Error out when wal_log_hints=off and no data checksums +# - Succeeds after reload to on +# +use strict; +use warnings FATAL => 'all'; +use PostgreSQL::Test::Cluster; +use PostgreSQL::Test::Utils; +use Test::More; + +# Initialize primary without data checksums and with wal_log_hints=off +my $node_primary = PostgreSQL::Test::Cluster->new('primary'); +$node_primary->init(allows_streaming => 1, no_data_checksums => 1); +$node_primary->append_conf( + 'postgresql.conf', qq{ +wal_log_hints = off +wal_level = replica +wal_keep_size = 64MB +autovacuum = off +}); +$node_primary->start; + +$node_primary->safe_psql('postgres', + "CREATE TABLE t(id int); INSERT INTO t SELECT generate_series(1,100)"); +$node_primary->safe_psql('postgres', "CHECKPOINT"); + +# Create standby, diverge +my $node_standby = PostgreSQL::Test::Cluster->new('standby'); +$node_primary->backup('my_backup'); +$node_standby->init_from_backup($node_primary, 'my_backup', + has_streaming => 1); +$node_standby->start; +$node_primary->wait_for_catchup($node_standby); + +$node_standby->promote; +$node_standby->safe_psql('postgres', "INSERT INTO t VALUES (999)"); +$node_primary->safe_psql('postgres', "INSERT INTO t VALUES (888)"); +$node_primary->safe_psql('postgres', "CHECKPOINT"); +$node_primary->stop; + +# pg_rewind must refuse: wal_log_hints=off +command_fails_like( + [ + 'pg_rewind', + '--target-pgdata' => $node_primary->data_dir, + '--source-server' => $node_standby->connstr('postgres'), + '--no-sync', + ], + qr/target server needs to use either data checksums or "wal_log_hints = on"/, + 'pg_rewind refuses with wal_log_hints=off and no data checksums'); + +# Restart primary and enable wal_log_hints via reload +$node_primary->start; +$node_primary->append_conf('postgresql.conf', 'wal_log_hints = on'); +$node_primary->reload; + +my $result = $node_primary->safe_psql('postgres', "SHOW wal_log_hints"); +is($result, 'on', 'wal_log_hints changed to on via reload'); + +$node_primary->stop; + +# Same pg_rewind now succeeds +command_ok( + [ + 'pg_rewind', + '--target-pgdata' => $node_primary->data_dir, + '--source-server' => $node_standby->connstr('postgres'), + '--no-sync', + ], + 'pg_rewind succeeds after enabling wal_log_hints via reload'); + +$node_standby->stop; +done_testing(); diff --git a/src/include/access/xlog.h b/src/include/access/xlog.h index 4dd98624204..8016eaadb53 100644 --- a/src/include/access/xlog.h +++ b/src/include/access/xlog.h @@ -120,7 +120,7 @@ extern PGDLLIMPORT bool XLogLogicalInfo; * of the bits make it to disk, but the checksum wouldn't match. Also WAL-log * them if forced by wal_log_hints=on. */ -#define XLogHintBitIsNeeded() (wal_log_hints || DataChecksumsNeedWrite()) +extern bool XLogHintBitIsNeeded(void); /* Do we need to WAL-log information required only for Hot Standby and logical replication? */ #define XLogStandbyInfoActive() (wal_level >= WAL_LEVEL_REPLICA) @@ -274,6 +274,7 @@ extern void XLogPutNextOid(Oid nextOid); extern XLogRecPtr XLogRestorePoint(const char *rpName); extern XLogRecPtr XLogAssignLSN(void); extern void UpdateFullPageWrites(void); +extern void UpdateWalLogHints(void); extern void GetFullPageWriteInfo(XLogRecPtr *RedoRecPtr_p, bool *doPageWrites_p); extern XLogRecPtr GetRedoRecPtr(void); extern XLogRecPtr GetInsertRecPtr(void); -- 2.34.1 ^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: Allow wal_log_hints to be changed without restart @ 2026-07-20 04:24 Tatsuya Kawata <kawatatatsuya0913@gmail.com> parent: Bertrand Drouvot <bertranddrouvot.pg@gmail.com> 0 siblings, 0 replies; 3+ messages in thread From: Tatsuya Kawata @ 2026-07-20 04:24 UTC (permalink / raw) To: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>; +Cc: pgsql-hackers@lists.postgresql.org 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] 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] already suggested on the pg_rewind side, which does not seem to have been implemented. [1] https://postgr.es/m/CAB7nPqSXYs4Jg-KJMy8xiM4sTqkgKvHyDrCoojhXzRSKiW57+g@mail.gmail.com Regards, Tatsuya Kawata ^ permalink raw reply [nested|flat] 3+ messages in thread
end of thread, other threads:[~2026-07-20 04:24 UTC | newest] Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2026-06-13 07:20 [PATCH v1] Allow wal_log_hints to be changed without restart Bertrand Drouvot <bertranddrouvot.pg@gmail.com> 2026-06-15 13:40 Allow wal_log_hints to be changed without restart Bertrand Drouvot <bertranddrouvot.pg@gmail.com> 2026-07-20 04:24 ` Tatsuya Kawata <kawatatatsuya0913@gmail.com>
This inbox is served by DDX for PostgreSQL; see mirroring instructions for how to clone and mirror all data and code used for this inbox