At 2026-08-15 22:30:56, "Zsolt Parragi" <[email protected]> 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
