From b41df0872b8d816aca6b777d30cfe9cbb6822770 Mon Sep 17 00:00:00 2001 From: Zsolt Parragi Date: Sat, 15 Aug 2026 07:45:22 +0000 Subject: [PATCH] basebackup: do not verify checksums on pages from before enabling Enabling data checksums in a running cluster changes the cluster state to "on" before the checkpoint which flushes the pages the worker rewrote. A base backup which started before that transition absorbs the barrier mid-run and starts verifying pages whose on-disk copies legitimately lack checksums, and which carry LSNs older than the backup start so the LSN check does not skip them either. The backup fails with bogus corruption warnings and inflates the checksum failure counters. Backups from a standby have an even larger window since nothing forces a restartpoint after the state change. The same applies to checksums being disabled and re-enabled while the backup runs: hint bits set while checksums were off reach disk without a checksum update and without moving the page LSN, and once the re-enabling completes the per-page state checks would resume verifying and trip over such pages. To fix, verify checksums only while they have been continuously enabled since the checkpoint the backup started from. Track the location of the last XLOG2_CHECKSUMS record inserted or replayed, and verify only when the state is "on" and the last change predates the backup start: the starting checkpoint then guarantees that every page flushed before it has a checksum written, and any later change disables verification for the rest of the backup. Both tests need a shared buffer pool large enough to keep the rewritten pages dirty until the final checkpoint, otherwise they are written out with checksums before the backup reads them. The re-enable test must also keep wal_log_hints off, since logging the hint bit updates would move the page LSNs past the backup start and the LSN check would skip the pages anyway. --- src/backend/access/transam/xlog.c | 36 ++++++ src/backend/backup/basebackup.c | 45 +++++-- src/include/access/xlog.h | 1 + src/test/modules/test_checksums/meson.build | 2 + .../test_checksums/t/010_backup_straddle.pl | 105 ++++++++++++++++ .../test_checksums/t/011_backup_onoffon.pl | 119 ++++++++++++++++++ 6 files changed, 298 insertions(+), 10 deletions(-) create mode 100644 src/test/modules/test_checksums/t/010_backup_straddle.pl create mode 100644 src/test/modules/test_checksums/t/011_backup_onoffon.pl diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index f8b939853e9..75c05c1bdab 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -559,6 +559,14 @@ typedef struct XLogCtlData /* last data_checksum_version we've seen */ uint32 data_checksum_version; + /* + * lastChecksumChangeRecPtr points to the end of the last XLOG2_CHECKSUMS + * record inserted or replayed, i.e. the last change of + * data_checksum_version. InvalidXLogRecPtr if the state hasn't changed + * since the server started. + */ + XLogRecPtr lastChecksumChangeRecPtr; + slock_t info_lck; /* locks shared variables shown above */ } XLogCtlData; @@ -4734,6 +4742,27 @@ DataChecksumsNeedVerify(void) return (LocalDataChecksumState == PG_DATA_CHECKSUM_VERSION); } +/* + * GetLastChecksumChangeRecPtr + * Returns the location of the last data checksum state change + * + * Returns the end of the last XLOG2_CHECKSUMS record inserted or replayed + * since the server started, or InvalidXLogRecPtr if there was none. Note + * that offline state changes by pg_checksums leave no trace here; callers + * comparing against it must also inspect the current state. + */ +XLogRecPtr +GetLastChecksumChangeRecPtr(void) +{ + XLogRecPtr ptr; + + SpinLockAcquire(&XLogCtl->info_lck); + ptr = XLogCtl->lastChecksumChangeRecPtr; + SpinLockRelease(&XLogCtl->info_lck); + + return ptr; +} + /* * SetDataChecksumsOnInProgress * Sets the data checksum state to "inprogress-on" to enable checksums @@ -4844,6 +4873,8 @@ SetDataChecksumsOn(void) MyProc->delayChkptFlags &= ~DELAY_CHKPT_START; END_CRIT_SECTION(); + INJECTION_POINT("datachecksums-on-before-checkpoint", NULL); + RequestCheckpoint(CHECKPOINT_FORCE | CHECKPOINT_WAIT | CHECKPOINT_FAST); WaitForProcSignalBarrier(barrier); } @@ -8742,6 +8773,10 @@ XLogChecksums(uint32 new_type) recptr = XLogInsert(RM_XLOG2_ID, XLOG2_CHECKSUMS); XLogFlush(recptr); + + SpinLockAcquire(&XLogCtl->info_lck); + XLogCtl->lastChecksumChangeRecPtr = recptr; + SpinLockRelease(&XLogCtl->info_lck); } /* @@ -9247,6 +9282,7 @@ xlog2_redo(XLogReaderState *record) SpinLockAcquire(&XLogCtl->info_lck); XLogCtl->data_checksum_version = state.new_checksum_state; + XLogCtl->lastChecksumChangeRecPtr = record->EndRecPtr; SpinLockRelease(&XLogCtl->info_lck); LWLockAcquire(ControlFileLock, LW_EXCLUSIVE); diff --git a/src/backend/backup/basebackup.c b/src/backend/backup/basebackup.c index fe5ce23aaba..0de44e08ed7 100644 --- a/src/backend/backup/basebackup.c +++ b/src/backend/backup/basebackup.c @@ -107,6 +107,7 @@ static off_t read_file_data_into_buffer(bbsink *sink, int *checksum_failures); static void push_to_sink(bbsink *sink, pg_checksum_context *checksum_ctx, size_t *bytes_done, void *data, size_t length); +static bool backup_checksums_verifiable(XLogRecPtr start_lsn); static bool verify_page_checksum(Page page, XLogRecPtr start_lsn, BlockNumber blkno, uint16 *expected_checksum); @@ -1609,13 +1610,14 @@ sendFile(bbsink *sink, const char *readfilename, const char *tarfilename, Assert((sink->bbs_buffer_length % BLCKSZ) == 0); /* - * If we weren't told not to verify checksums, and if checksums are - * enabled for this cluster, and if this is a relation file, then verify - * the checksum. We cannot at this point check if checksums are enabled - * or disabled as that might change, thus we check at each point where we - * could be validating a checksum. + * If we weren't told not to verify checksums, and if checksums have been + * continuously enabled since the checkpoint this backup started from, and + * if this is a relation file, then verify the checksum. Checksums can + * still be disabled while the backup runs, thus we check at each point + * where we could be validating a checksum. */ - if (!noverify_checksums && RelFileNumberIsValid(relfilenumber)) + if (!noverify_checksums && RelFileNumberIsValid(relfilenumber) && + backup_checksums_verifiable(sink->bbs_state->startptr)) verify_checksum = true; /* @@ -1748,7 +1750,9 @@ sendFile(bbsink *sink, const char *readfilename, const char *tarfilename, * If the amount of data we were able to read was not a multiple of * BLCKSZ, we cannot verify checksums, which are block-level. */ - if (verify_checksum && DataChecksumsNeedVerify() && (cnt % BLCKSZ != 0)) + if (verify_checksum && + backup_checksums_verifiable(sink->bbs_state->startptr) && + (cnt % BLCKSZ != 0)) { ereport(WARNING, (errmsg("could not verify checksum in file \"%s\", block " @@ -1876,7 +1880,7 @@ read_file_data_into_buffer(bbsink *sink, const char *readfilename, int fd, * The data checksum state can change at any point, so we need to * re-check before each page. */ - if (!DataChecksumsNeedVerify()) + if (!backup_checksums_verifiable(sink->bbs_state->startptr)) return cnt; page = sink->bbs_buffer + BLCKSZ * i; @@ -1905,7 +1909,7 @@ read_file_data_into_buffer(bbsink *sink, const char *readfilename, int fd, * The data checksum state may also have changed concurrently so check * again. */ - if (!DataChecksumsNeedVerify()) + if (!backup_checksums_verifiable(sink->bbs_state->startptr)) return cnt; reread_cnt = basebackup_read_file(fd, sink->bbs_buffer + BLCKSZ * i, @@ -1996,6 +2000,27 @@ push_to_sink(bbsink *sink, pg_checksum_context *checksum_ctx, } } +/* + * Check whether data checksums can be verified for a backup started at + * start_lsn. + * + * Checksums are verified only while they have been continuously enabled + * since the checkpoint the backup started from: the state must be "on" and + * the last state change must predate the backup start. Such a checkpoint + * guarantees that every page flushed before it has a checksum written. Any + * later state change ends verification for the rest of the backup: pages + * written while checksums were off can lack checksums yet keep LSNs older + * than the backup start, and re-enabling completes before the rewritten + * pages are flushed, so observing the "on" state again is not enough to + * resume. + */ +static bool +backup_checksums_verifiable(XLogRecPtr start_lsn) +{ + return DataChecksumsNeedVerify() && + GetLastChecksumChangeRecPtr() <= start_lsn; +} + /* * Try to verify the checksum for the provided page, if it seems appropriate * to do so. @@ -2021,7 +2046,7 @@ verify_page_checksum(Page page, XLogRecPtr start_lsn, BlockNumber blkno, if (PageIsNew(page) || PageGetLSN(page) >= start_lsn) return true; - if (!DataChecksumsNeedVerify()) + if (!backup_checksums_verifiable(start_lsn)) return true; /* Perform the actual checksum calculation. */ diff --git a/src/include/access/xlog.h b/src/include/access/xlog.h index 4dd98624204..8a22314fb6f 100644 --- a/src/include/access/xlog.h +++ b/src/include/access/xlog.h @@ -249,6 +249,7 @@ extern uint64 GetSystemIdentifier(void); extern char *GetMockAuthenticationNonce(void); extern bool DataChecksumsNeedWrite(void); extern bool DataChecksumsNeedVerify(void); +extern XLogRecPtr GetLastChecksumChangeRecPtr(void); extern bool DataChecksumsOn(void); extern bool DataChecksumsOff(void); extern bool DataChecksumsInProgressOn(void); diff --git a/src/test/modules/test_checksums/meson.build b/src/test/modules/test_checksums/meson.build index 9b1421a9b91..bbb2f1e9047 100644 --- a/src/test/modules/test_checksums/meson.build +++ b/src/test/modules/test_checksums/meson.build @@ -33,6 +33,8 @@ tests += { 't/007_pgbench_standby.pl', 't/008_pitr.pl', 't/009_fpi.pl', + 't/010_backup_straddle.pl', + 't/011_backup_onoffon.pl', ], }, } diff --git a/src/test/modules/test_checksums/t/010_backup_straddle.pl b/src/test/modules/test_checksums/t/010_backup_straddle.pl new file mode 100644 index 00000000000..6471c3507a9 --- /dev/null +++ b/src/test/modules/test_checksums/t/010_backup_straddle.pl @@ -0,0 +1,105 @@ + +# Copyright (c) 2026, PostgreSQL Global Development Group + +# Test a base backup which is running when enabling data checksums completes. +# The transition to "on" happens before the checkpoint which flushes the +# rewritten pages, so such a backup reads on-disk pages which legitimately +# lack checksums and carry LSNs older than the backup start. + +use strict; +use warnings FATAL => 'all'; + +use PostgreSQL::Test::Cluster; +use PostgreSQL::Test::Utils; +use Test::More; +use IPC::Run; + +use FindBin; +use lib $FindBin::RealBin; + +use DataChecksums::Utils; + +if ($ENV{enable_injection_points} ne 'yes') +{ + plan skip_all => 'Injection points not supported by this build'; +} + +my $node = PostgreSQL::Test::Cluster->new('straddle_node'); +$node->init(no_data_checksums => 1, allows_streaming => 1); +# The pages rewritten while enabling must stay dirty in shared buffers until +# the final checkpoint, otherwise they reach disk with checksums on their own +# and nothing is left to misjudge. +$node->append_conf('postgresql.conf', 'shared_buffers = 128MB'); +$node->start; + +$node->safe_psql('postgres', 'CREATE EXTENSION injection_points;'); + +# Enough data to keep the throttled backup running while checksums are being +# enabled. The scan pulls the table into shared buffers so that enabling +# doesn't read it through a ring buffer, which would write the pages back out. +$node->safe_psql('postgres', + "CREATE TABLE t AS SELECT generate_series(1,600000) AS a;"); +$node->safe_psql('postgres', "SELECT count(*) FROM t;"); +test_checksum_state($node, 'off'); + +# Hold the transition after the state changed to "on" but before the +# checkpoint which flushes the rewritten pages. +$node->safe_psql('postgres', + "SELECT injection_points_attach('datachecksums-on-before-checkpoint','wait');" +); + +my $backupdir = $node->backup_dir . '/straddle'; +my ($out, $err) = ('', ''); +my $backup = IPC::Run::start( + [ + 'pg_basebackup', '-D', $backupdir, + '--wal-method=none', '--no-sync', + '--checkpoint=fast', '--max-rate=4096', + '-d', $node->connstr('postgres') + ], + '>', \$out, '2>', \$err, + IPC::Run::timeout(180)); + +$node->poll_query_until('postgres', + "SELECT count(*) > 0 FROM pg_catalog.pg_stat_activity " + . "WHERE backend_type = 'walsender' AND state = 'active';"); + +enable_data_checksums($node); +$node->wait_for_event('datachecksums launcher', + 'datachecksums-on-before-checkpoint'); + +# The backup must still be sending files, otherwise it never sees the state +# change and the test is pointless. +my $running = $node->safe_psql('postgres', + "SELECT count(*) FROM pg_catalog.pg_stat_activity " + . "WHERE backend_type = 'walsender';"); +is($running, '1', 'backup still running when checksums were enabled'); + +ok($backup->finish, 'backup straddling enable completion succeeds') + or diag("stderr: $err"); + +$node->safe_psql('postgres', + "SELECT injection_points_wakeup('datachecksums-on-before-checkpoint');"); +$node->safe_psql('postgres', + "SELECT injection_points_detach('datachecksums-on-before-checkpoint');"); + +wait_for_checksum_state($node, 'on'); +$node->poll_query_until('postgres', + "SELECT count(*) = 0 FROM pg_catalog.pg_stat_activity " + . "WHERE backend_type = 'datachecksums launcher';"); + +my $result = $node->safe_psql('postgres', + "SELECT coalesce(sum(checksum_failures), 0) FROM pg_catalog.pg_stat_database;" +); +is($result, '0', 'no spurious checksum failures reported'); + +# A backup started once enabling has completed must verify, and pass +$node->command_ok( + [ + 'pg_basebackup', '-D', $node->backup_dir . '/after', + '--wal-method=none', '--no-sync', '--checkpoint=fast' + ], + 'backup after enable completion succeeds'); + +$node->stop; +done_testing(); diff --git a/src/test/modules/test_checksums/t/011_backup_onoffon.pl b/src/test/modules/test_checksums/t/011_backup_onoffon.pl new file mode 100644 index 00000000000..f843491a36c --- /dev/null +++ b/src/test/modules/test_checksums/t/011_backup_onoffon.pl @@ -0,0 +1,119 @@ + +# Copyright (c) 2026, PostgreSQL Global Development Group + +# Test a base backup which is running while data checksums are disabled and +# then re-enabled. Hint bit updates made while checksums were off reach disk +# without a checksum update and without moving the page LSN, so once the +# re-enabling completes the backup would resume verification and misjudge +# those pages until the rewritten versions are flushed. + +use strict; +use warnings FATAL => 'all'; + +use PostgreSQL::Test::Cluster; +use PostgreSQL::Test::Utils; +use Test::More; +use IPC::Run; + +use FindBin; +use lib $FindBin::RealBin; + +use DataChecksums::Utils; + +if ($ENV{enable_injection_points} ne 'yes') +{ + plan skip_all => 'Injection points not supported by this build'; +} + +my $node = PostgreSQL::Test::Cluster->new('onoffon_node'); +$node->init(allows_streaming => 1); +# The pages rewritten while re-enabling must stay dirty in shared buffers +# until the final checkpoint, otherwise they reach disk with checksums on +# their own and nothing is left to misjudge. Autovacuum is disabled so that +# nothing sets hint bits before checksums are turned off, and wal_log_hints +# (implied by allows_streaming) must be off so that setting them does not +# move the page LSNs past the backup start. +$node->append_conf('postgresql.conf', 'shared_buffers = 128MB'); +$node->append_conf('postgresql.conf', 'autovacuum = off'); +$node->append_conf('postgresql.conf', 'wal_log_hints = off'); +$node->start; + +$node->safe_psql('postgres', 'CREATE EXTENSION injection_points;'); + +# Enough data to keep the throttled backup running while checksums are being +# turned off and back on. The table is not read here, leaving the hint bits +# unset until checksums are off. +$node->safe_psql('postgres', + "CREATE TABLE t AS SELECT generate_series(1,600000) AS a;"); +test_checksum_state($node, 'on'); + +my $backupdir = $node->backup_dir . '/onoffon'; +my ($out, $err) = ('', ''); +my $backup = IPC::Run::start( + [ + 'pg_basebackup', '-D', $backupdir, + '--wal-method=none', '--no-sync', + '--checkpoint=fast', '--max-rate=4096', + '-d', $node->connstr('postgres') + ], + '>', \$out, '2>', \$err, + IPC::Run::timeout(180)); + +$node->poll_query_until('postgres', + "SELECT count(*) > 0 FROM pg_catalog.pg_stat_activity " + . "WHERE backend_type = 'walsender' AND state = 'active';"); + +disable_data_checksums($node, wait => 1); + +# With checksums off, the scan sets hint bits without WAL logging them, and +# the checkpoint flushes the modified pages without updating their checksums. +# The on-disk pages now carry stale checksums and LSNs older than the backup +# start. +$node->safe_psql('postgres', "SELECT count(*) FROM t;"); +$node->safe_psql('postgres', "CHECKPOINT;"); + +# Hold the re-enabling after the state changed to "on" but before the +# checkpoint which flushes the rewritten pages. +$node->safe_psql('postgres', + "SELECT injection_points_attach('datachecksums-on-before-checkpoint','wait');" +); + +enable_data_checksums($node); +$node->wait_for_event('datachecksums launcher', + 'datachecksums-on-before-checkpoint'); + +# The backup must still be sending files, otherwise it never sees the state +# changes and the test is pointless. +my $running = $node->safe_psql('postgres', + "SELECT count(*) FROM pg_catalog.pg_stat_activity " + . "WHERE backend_type = 'walsender';"); +is($running, '1', 'backup still running when checksums were re-enabled'); + +ok($backup->finish, 'backup straddling disable and re-enable succeeds') + or diag("stderr: $err"); + +$node->safe_psql('postgres', + "SELECT injection_points_wakeup('datachecksums-on-before-checkpoint');"); +$node->safe_psql('postgres', + "SELECT injection_points_detach('datachecksums-on-before-checkpoint');"); + +wait_for_checksum_state($node, 'on'); +$node->poll_query_until('postgres', + "SELECT count(*) = 0 FROM pg_catalog.pg_stat_activity " + . "WHERE backend_type = 'datachecksums launcher';"); + +my $result = $node->safe_psql('postgres', + "SELECT coalesce(sum(checksum_failures), 0) FROM pg_catalog.pg_stat_database;" +); +is($result, '0', 'no spurious checksum failures reported'); + +# A backup started once re-enabling has completed must verify, and pass +$node->command_ok( + [ + 'pg_basebackup', '-D', $node->backup_dir . '/after', + '--wal-method=none', '--no-sync', '--checkpoint=fast' + ], + 'backup after re-enable completion succeeds'); + +$node->stop; +done_testing(); -- 2.54.0