basebackup: do not verify checksums on pages written before enabling checksums

On 14 Aug 2026, at 21:36, Zsolt Parragi <zsolt.parragi@percona.com> wrote:

I am not 100% happy with the shape of the patch, but I couldn't come
up with a better solution even after staring at it for a long time,
and I think it is at least correct and good enough. But maybe somebody
has a better idea for solving this problem.

Thanks! I wasn't able to come up with a mor elegant solution either when
staring at it but I'll take another look at it together with the other fixes
staged.

--
Daniel Gustafsson

Hi,

On Fri, Aug 14, 2026 at 09:57:32PM +0200, Daniel Gustafsson wrote:

On 14 Aug 2026, at 21:36, Zsolt Parragi <zsolt.parragi@percona.com> wrote:

Thanks for the report and the patch!

I am not 100% happy with the shape of the patch, but I couldn't come
up with a better solution even after staring at it for a long time,
and I think it is at least correct and good enough. But maybe somebody
has a better idea for solving this problem.

Thanks! I wasn't able to come up with a mor elegant solution either when
staring at it but I'll take another look at it together with the other fixes
staged.

I had a look and I wonder if the boolean could miss an on->off->on series of
changes. checksums_on_at_start would remain true in that case. Once checksums
are enabled again, we'd resume verification even though the final checkpoint may
still be running.

Maybe tracking a checksum transition generation/LSN would make this easier to
reason about? Or am I missing something preventing this case?

Regards,

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

I had a look and I wonder if the boolean could miss an on->off->on series of
changes.

Good catch, I missed that. I checked that on->off works, but
on->off->on reenables verification and can again see checksum errors.

Maybe tracking a checksum transition generation/LSN would make this easier to
reason about?

That seems like a good idea, that would allow us to remove the static
variable. v2 attached based on this approach. (the on-off-on issue can
also be fixed in the original static bool version with slightly more
changes, we could also go in that direction, but let's first see what
everyone thinks about this approach)

On 15 Aug 2026, at 10:10, Zsolt Parragi <zsolt.parragi@percona.com> wrote:

I had a look and I wonder if the boolean could miss an on->off->on series of
changes.

Good catch, I missed that. I checked that on->off works, but
on->off->on reenables verification and can again see checksum errors.

Maybe tracking a checksum transition generation/LSN would make this easier to
reason about?

That seems like a good idea, that would allow us to remove the static
variable. v2 attached based on this approach. (the on-off-on issue can
also be fixed in the original static bool version with slightly more
changes, we could also go in that direction, but let's first see what
everyone thinks about this approach)

I prefer this approach, getting rid of the static variable is a neat win. Can
we combine the two tests into a single 010_backup.pl to keep cluster inits
down?

+ * If we weren't told not to verify checksums, and if checksums have been

While not the fault of this patch, I think we should take this opportunity to
remove the double negative and rewrite this to "If we were told to verify
checksums".

--
Daniel Gustafsson

Hi,

On Sat, Aug 15, 2026 at 10:58:53AM +0200, Daniel Gustafsson wrote:

On 15 Aug 2026, at 10:10, Zsolt Parragi <zsolt.parragi@percona.com> wrote:

That seems like a good idea, that would allow us to remove the static
variable. v2 attached based on this approach.

Thanks!

I prefer this approach,

+1

A few comments:

=== 1

+        * 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;

I'm not sure that "since the server started" is enough for a backup taken from
a standby. XLogCtl is zeroed at startup, while the checksum state is restored
from pg_control. The standby can become available before replay reaches the
latest checksum transition, so verification could resume with
lastChecksumChangeRecPtr invalid.

I wonder if the transition LSN should survive a server restart?

=== 2

+GetLastChecksumChangeRecPtr(void)
+{
+       XLogRecPtr      ptr;
+
+       SpinLockAcquire(&XLogCtl->info_lck);
+       ptr = XLogCtl->lastChecksumChangeRecPtr;
+       SpinLockRelease(&XLogCtl->info_lck);

Then:

+backup_checksums_verifiable(XLogRecPtr start_lsn)
+{
+   return DataChecksumsNeedVerify() &&
+       GetLastChecksumChangeRecPtr() <= start_lsn;

and:

@@ -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))

and:

@@ -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))

This means two acquisitions of the spinlock per verified page.

I think that an atomic would make more sense. XLogCtlData already uses atomics,
and lastChecksumChangeRecPtr does not need to be read consistently with any other
field.

Regards,

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

Can we combine the two tests into a single 010_backup.pl to keep cluster
inits down?

I reworked the tests with another injection point as part of this,
this way they shouldn't be flaky on CI and they are significantly
faster now.

I'm not sure that "since the server started" is enough for a backup taken from
a standby. XLogCtl is zeroed at startup, while the checksum state is restored
from pg_control. The standby can become available before replay reaches the
latest checksum transition, so verification could resume with
lastChecksumChangeRecPtr invalid.

There was really an issue there, I think we can solve that by
advancing minrecoverypoint. I kept a test case for this separate for
now, as I am unsure if we want to include it in some form.

I think that an atomic would make more sense.

Also done.

On 15 Aug 2026, at 16:30, Zsolt Parragi <zsolt.parragi@percona.com> wrote:

Can we combine the two tests into a single 010_backup.pl to keep cluster
inits down?

I reworked the tests with another injection point as part of this,
this way they shouldn't be flaky on CI and they are significantly
faster now.

I'm not sure that "since the server started" is enough for a backup taken from
a standby. XLogCtl is zeroed at startup, while the checksum state is restored
from pg_control. The standby can become available before replay reaches the
latest checksum transition, so verification could resume with
lastChecksumChangeRecPtr invalid.

There was really an issue there, I think we can solve that by
advancing minrecoverypoint. I kept a test case for this separate for
now, as I am unsure if we want to include it in some form.

I think that an atomic would make more sense.

Also done.

Thanks for the update! I am travelling today but will take a closer look
tomorrow evening when I am back again.

--
Daniel Gustafsson

Hi,

On Sat, Aug 15, 2026 at 03:30:56PM +0100, Zsolt Parragi wrote:

Can we combine the two tests into a single 010_backup.pl to keep cluster
inits down?

I reworked the tests with another injection point as part of this,
this way they shouldn't be flaky on CI and they are significantly
faster now.

I'm not sure that "since the server started" is enough for a backup taken from
a standby. XLogCtl is zeroed at startup, while the checksum state is restored
from pg_control. The standby can become available before replay reaches the
latest checksum transition, so verification could resume with
lastChecksumChangeRecPtr invalid.

There was really an issue there, I think we can solve that by
advancing minrecoverypoint.

Yeah, that works too. IIUC we advance minRecoveryPoint for every checksum state
record. I wonder if we could do so only when necessary, means when verification
change (inprogress-on->on and on->inprogress-off). That said that's a nit as
advancing it for every transition is simpler and less error prone.

=== 1

+my $enable_start_lsn =
+  $node_primary->safe_psql('postgres', 'SELECT pg_current_wal_insert_lsn();');
bla
bla
+enable_data_checksums($node_primary);

So enable_start_lsn is recorded before enable starts.

And:

+       "SELECT '$min_recovery'::pg_lsn > '$enable_start_lsn'::pg_lsn;"),
+   't',
+   'minRecoveryPoint advanced past the checksum state change');

Since enabling emits both inprogress-on and on records, this would pass even if
only the first one advanced minRecoveryPoint.

If we want to check that minRecoveryPoint >= final on record, could the test use
datachecksums-enable-checksums-delay to pause before that transition and establish
the restartpoint there?

=== 2

+# A backup started once enabling has completed must verify, and pass
+$node->command_ok(
+   [
+       'pg_basebackup', '-D', $node->backup_dir . '/after_enable',
+       '--wal-method=none', '--no-sync', '--checkpoint=fast'
+   ],
+   'backup after enable completion succeeds');

I think that only prove that no false checksum failure is reported.
Could one post transition backup reuse the existing corruption mechanism (see
010_pg_basebackup.pl) to check that verification resumes?

=== 3

011_standby_straddle.pl does:

+bgwriter_lru_maxpages = 0

but 010_backup_straddle.pl does not. Should 010_backup_straddle.pl also disable
bgwriter to preserve dirty pages?

Regards,

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

At 2026-08-15 22:30:56, "Zsolt Parragi" <zsolt.parragi@percona.com> wrote:

I reworked the tests with another injection point as part of this,
this way they shouldn't be flaky on CI and they are significantly
faster now.

I'm not sure that "since the server started" is enough for a backup taken from
a standby. XLogCtl is zeroed at startup, while the checksum state is restored
from pg_control. The standby can become available before replay reaches the
latest checksum transition, so verification could resume with
lastChecksumChangeRecPtr invalid.

There was really an issue there, I think we can solve that by
advancing minrecoverypoint. I kept a test case for this separate for
now, as I am unsure if we want to include it in some form.

I think that an atomic would make more sense.

Also done.

Hi,
I have several review comments for patch v3.

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..db50cd81fcd
--- /dev/null
+++ b/src/test/modules/test_checksums/t/010_backup_straddle.pl
+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 after enable');

1.
For regression‑test scenarios simulating the "exactly‑one‑page‑failure‑per‑file" fault, there is a blind spot in the counter.
The final ERROR on total_checksum_failures in basebackup.c will still abort the backup, but this check can become ineffective.

diff --git a/src/backend/backup/basebackup.c b/src/backend/backup/basebackup.c
index fe5ce23aaba..3e00cd0dd6e 100644
--- a/src/backend/backup/basebackup.c
+++ b/src/backend/backup/basebackup.c
+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 +2053,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. */
2.
In this scenario, if a user starts a backup a few seconds before enable completes, the entire backup skips all page checksums.
Users will obtain a backup with zero checksum validation, while believing checksum verification is enabled.

Best regards,

--

Yilin Zhang

On 17 Aug 2026, at 05:07, Bertrand Drouvot <bertranddrouvot.pg@gmail.com> wrote:
On Sat, Aug 15, 2026 at 03:30:56PM +0100, Zsolt Parragi wrote:

There was really an issue there, I think we can solve that by
advancing minrecoverypoint.

Yeah, that works too.

Agreed, and there is prior art for this solution.

IIUC we advance minRecoveryPoint for every checksum state
record. I wonder if we could do so only when necessary, means when verification
change (inprogress-on->on and on->inprogress-off). That said that's a nit as
advancing it for every transition is simpler and less error prone.

I think we should still to doing it for every transition in 19, and if someone
feels like optimizing it can be revisited for 20.

If we want to check that minRecoveryPoint >= final on record, could the test use
datachecksums-enable-checksums-delay to pause before that transition and establish
the restartpoint there?

I tried that in the attached 0002. The risk I see is that it may become flaky
if we don't set about waiting for the next event in time after waking up this
wait. Might not be a problem but the buildfarm has a tendency to expose a lof
of behavior.

I think that only prove that no false checksum failure is reported.
Could one post transition backup reuse the existing corruption mechanism (see
010_pg_basebackup.pl) to check that verification resumes?

Good idea, also done in 0002.

011_standby_straddle.pl does:

+bgwriter_lru_maxpages = 0

but 010_backup_straddle.pl does not. Should 010_backup_straddle.pl also disable
bgwriter to preserve dirty pages?

I'm not sure, does it need to?

Another thing I am pondering is to place these tests under PG_TEST_EXTRA.
While 128MB for shared_buffers is pretty moderate, I'm not sure we want to
induce that on a normal BF run. 0002 does some backup cleaning as well as a
pgperltidy and pgindent and some very minor fiddling with test code.

--
Daniel Gustafsson

On 17 Aug 2026, at 10:36, Yilin Zhang <jiezhilove@126.com> wrote:

For regression‑test scenarios simulating the "exactly‑one‑page‑failure‑per‑file" fault, there is a blind spot in the counter.
The final ERROR on total_checksum_failures in basebackup.c will still abort the backup, but this check can become ineffective.

I might be missing what you are referring to, isn't the check for the backup
command not failing catching this?

In this scenario, if a user starts a backup a few seconds before enable completes, the entire backup skips all page checksums.
Users will obtain a backup with zero checksum validation, while believing checksum verification is enabled.

There is little we can do though isn't there? If a backup completes seconds
before inserting very important data then that data isn't backed up, we cannot
foresee what the user might do and we cannot foresee a checksum enabling
finishing in time so we should wait. The effects on verification during a
backup should however be documented.

--
Daniel Gustafsson

I think we should still to doing it for every transition in 19, and if someone
feels like optimizing it can be revisited for 20.

Agree, this is the simplest/safest choice for now.

Another thing I am pondering is to place these tests under PG_TEST_EXTRA.
While 128MB for shared_buffers is pretty moderate, I'm not sure we want to
induce that on a normal BF run. 0002 does some backup cleaning as well as a
pgperltidy and pgindent and some very minor fiddling with test code.

If we include 011, that definitely should go into extra as that's also slow. 010 is at least relatively quick in its current form. Also 128 is just a quick "should be large enough" guess, 32mb also seems to be enough for 010, it still triggers the failure without the fix.

Users will obtain a backup with zero checksum validation, while believing checksum verification is enabled.

Why would they believe that it has checksums enabled, if they started the backup before checksums completed?

but 010_backup_straddle.pl does not. Should 010_backup_straddle.pl also disable
bgwriter to preserve dirty pages?

I'm not sure, does it need to?

I think the test should be safe as-is, since we have more than enough shared buffers. Even with 32mb shared buffers, the test reliably errors out without the fix.

At 2026-08-17 17:07:26, "Daniel Gustafsson" <daniel@yesql.se> wrote:

On 17 Aug 2026, at 10:36, Yilin Zhang <jiezhilove(at)126(dot)com> wrote:

For regression‑test scenarios simulating the "exactly‑one‑page‑failure‑per‑file" fault, there is a blind spot in the counter.
The final ERROR on total_checksum_failures in basebackup.c will still abort the backup, but this check can become ineffective.

I might be missing what you are referring to, isn't the check for the backup
command not failing catching this?

In the "exactly‑one‑corrupted‑page‑per‑file" scenario, the backup fails.
The failure is triggered at the end of perform_base_backup() when total_checksum_failures equals 1,
via ereport(ERROR, errcode(XX001)).
However, pg_stat_database.checksum_failures is not updated at all.

Failure cases that can be captured by pg_stat_database.checksum_failures (≥2 corrupted pages per file) invariably result in backup failure:
total_checksum_failures becomes greater than zero and triggers an unconditional ERROR.
Judging by the current test cases,,
pg_stat_database.checksum_failures appears to offer no detection capability beyond what ok($backup->finish) already provides.

In this scenario, if a user starts a backup a few seconds before enable completes, the entire backup skips all page checksums.
Users will obtain a backup with zero checksum validation, while believing checksum verification is enabled.

There is little we can do though isn't there? If a backup completes seconds
before inserting very important data then that data isn't backed up, we cannot
foresee what the user might do and we cannot foresee a checksum enabling
finishing in time so we should wait. The effects on verification during a
backup should however be documented.
--
Daniel Gustafsson

Yes, either a warning or a documentation note would be fine.

Best regards,

--

Yilin Zhang

The failure is triggered at the end of perform_base_backup() when total_checksum_failures equals 1,
via ereport(ERROR, errcode(XX001)).
However, pg_stat_database.checksum_failures is not updated at all.

This seems like a separate issue existing for much longer, I'll submit
a separate patch about it.

Judging by the current test cases,,
pg_stat_database.checksum_failures appears to offer no detection capability beyond what ok($backup->finish) already provides.

I improved the tests a bit, while also adding the 32m shared buffers
change to 010. I kept this as a separate commit for easier review.

Hi,

On Mon, Aug 17, 2026 at 11:03:31AM +0200, Daniel Gustafsson wrote:

On 17 Aug 2026, at 05:07, Bertrand Drouvot <bertranddrouvot.pg@gmail.com> wrote:
On Sat, Aug 15, 2026 at 03:30:56PM +0100, Zsolt Parragi wrote:

I think we should still to doing it for every transition in 19, and if someone
feels like optimizing it can be revisited for 20.

Yeah, makes sense.

I tried that in the attached 0002.

That looks ok, thanks!

The risk I see is that it may become flaky
if we don't set about waiting for the next event in time after waking up this
wait.

The launcher stays blocked and keeps reporting the event until explicitly woken,
so I'm not sure I get your point.

I think that only prove that no false checksum failure is reported.
Could one post transition backup reuse the existing corruption mechanism (see
010_pg_basebackup.pl) to check that verification resumes?

Good idea, also done in 0002.

+$node->stop;
+$node->corrupt_page_checksum($fcorrupt, 0);
+$node->start;

The test stops and restarts the node before taking the backup, which resets
lastChecksumChangeRecPtr.

Should we also test that verification resumes while lastChecksumChangeRecPtr is
still set?

Regards,

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

Hi,

On Mon, Aug 17, 2026 at 03:36:03AM -0700, Zsolt Parragi wrote:

but 010_backup_straddle.pl does not. Should 010_backup_straddle.pl also disable
bgwriter to preserve dirty pages?

I'm not sure, does it need to?

I think the test should be safe as-is, since we have more than enough
shared buffers. Even with 32mb shared buffers, the test reliably
errors out without the fix.

Couldn't the bgwriter still flush those dirty pages, regardless of the shared
buffers size? On a slower BF animal, the launcher may remain paused long enough
for additional bgwriter cycles to flush the pages. I'd still set
bgwriter_lru_maxpages = 0 to make the test more deterministic.

Regards,

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

On 17 Aug 2026, at 14:54, Bertrand Drouvot <bertranddrouvot.pg@gmail.com> wrote:

+$node->stop;
+$node->corrupt_page_checksum($fcorrupt, 0);
+$node->start;

The test stops and restarts the node before taking the backup, which resets
lastChecksumChangeRecPtr.

Should we also test that verification resumes while lastChecksumChangeRecPtr is
still set?

corrupt_page_checksum() only works reliably when the cluster is turned off, a
reliable corrupt-checksum mechanism for an online cluster could for sure be
done but seems a bit more invasive at this point. Or am I missing something
obvious?

--
Daniel Gustafsson

Hi,

On Mon, Aug 17, 2026 at 03:15:43PM +0200, Daniel Gustafsson wrote:

On 17 Aug 2026, at 14:54, Bertrand Drouvot <bertranddrouvot.pg@gmail.com> wrote:

+$node->stop;
+$node->corrupt_page_checksum($fcorrupt, 0);
+$node->start;

The test stops and restarts the node before taking the backup, which resets
lastChecksumChangeRecPtr.

Should we also test that verification resumes while lastChecksumChangeRecPtr is
still set?

corrupt_page_checksum() only works reliably when the cluster is turned off,

Oh right, Doh, I missed that.

a
reliable corrupt-checksum mechanism for an online cluster could for sure be
done but seems a bit more invasive at this point.

Yeah. Maybe one option could be to add an injection point in verify_page_checksum(),
just after:

if (!backup_checksums_verifiable(start_lsn))
return true;

and before pg_checksum_page(), then attach an "error" action for the post transition
backup? That would cover verification resuming while lastChecksumChangeRecPtr is
still set, if we think it is worth testing.

Regards,

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

On 17 Aug 2026, at 14:52, Zsolt Parragi <zsolt.parragi@percona.com> wrote:

I improved the tests a bit, while also adding the 32m shared buffers

Thanks. I've squashed these patches and added PG_TEST_EXTRA gates as well as a
note in the base backup documentation about how checksum verification is
impacted in case enabling is in progress.

--
Daniel Gustafsson