Hi,
While checking commit 51f55b13a4d, I noticed a regression in repeated
calls to pg_enable_data_checksums().
Before that commit, calling pg_enable_data_checksums() after checksums had
reached the "on" state succeeded as a no-op. The existing basic TAP test
also documents that behavior.
51f55b13a4d added ErrorOnInvalidDatabases() before
StartDataChecksumsWorkerLauncher(). If an invalid database exists, the
same repeated call now fails even though no database processing is needed:
ERROR: cannot enable data checksums in a cluster with invalid database
"baddb"
The attached v1 patch restores that no-op, but does not use a bare
DataChecksumsOn() early return. Such a return could lose a pending disable
request and violate the existing last-request-wins behavior.
Instead, the patch makes the no-op decision under
DataChecksumsWorkerLock and records the requested enable operation and cost
settings atomically. If a disable operation is already running, it keeps
the invalid-database preflight because the subsequent enable may need to
process every database.
The patch adds two tests. The basic test covers repeated enable with an
invalid database while checksums are on, followed by a real enable from the
off state which must still fail. An injection test pauses an active disable
before the visible state changes from on, and verifies that enable still
checks invalid databases in that case.
Tested on origin/master at
33b392eaabdd1c563d40388784df051821e03c6b:
Unpatched master with the new basic test:
expected FAIL, 1 of 24 tests failed, only the new no-op assertion.
Patched default test_checksums suite:
PASS, 9 files and 95 tests.
Patched extended pgbench, standby, and PITR tests:
PASS, 3 files and 102 tests.
Full build, git diff --check, patch apply check, and Perl syntax:
PASS.
I tested master only. My local REL_19_STABLE ref predates the backpatch of
51f55b13a4d, and I did not refresh it while working offline.
This should be backpatched to v19 together with 51f55b13a4d.
Regards,
Ilmar Yunusov
From 7eba3fb96c6ad025a9c1673f17c339b38c39f857 Mon Sep 17 00:00:00 2001
From: Ilmar Yunusov <[email protected]>
Date: Wed, 29 Jul 2026 15:55:17 +0500
Subject: [PATCH v1] Restore idempotency of pg_enable_data_checksums()
Commit 51f55b13a4d made ErrorOnInvalidDatabases() run before
StartDataChecksumsWorkerLauncher(). Consequently, a repeated call to
pg_enable_data_checksums() fails when an invalid database exists, even
when checksums are already enabled and no processing is required.
Handle the no-op case while holding DataChecksumsWorkerLock. Record the
enable request and cost settings atomically so pending disable requests
retain last-request-wins semantics. If a disable operation is already
running, keep the invalid-database preflight because re-enabling may
require processing databases.
Extend the TAP tests to cover both a repeated enable with an invalid
database and an enable attempted while disabling is already running.
Backpatch to v19, where 51f55b13a4d was applied.
---
src/backend/postmaster/datachecksum_state.c | 20 ++++++++
.../modules/test_checksums/t/001_basic.pl | 14 ++++--
.../modules/test_checksums/t/005_injection.pl | 48 +++++++++++++++++++
3 files changed, 79 insertions(+), 3 deletions(-)
diff --git a/src/backend/postmaster/datachecksum_state.c b/src/backend/postmaster/datachecksum_state.c
index fc082ac37b9..7d91948edde 100644
--- a/src/backend/postmaster/datachecksum_state.c
+++ b/src/backend/postmaster/datachecksum_state.c
@@ -584,6 +584,25 @@ enable_data_checksums(PG_FUNCTION_ARGS)
errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("cost limit must be greater than zero"));
+ /*
+ * If checksums are already on and no disable operation is running, this
+ * request can be satisfied without processing any databases. Record the
+ * desired operation while holding the lock, so a pending disable is either
+ * canceled here or ordered after this request.
+ */
+ LWLockAcquire(DataChecksumsWorkerLock, LW_EXCLUSIVE);
+ if (DataChecksumsOn() &&
+ (!DataChecksumState->launcher_running ||
+ DataChecksumState->operation == ENABLE_DATACHECKSUMS))
+ {
+ DataChecksumState->launch_operation = ENABLE_DATACHECKSUMS;
+ DataChecksumState->launch_cost_delay = cost_delay;
+ DataChecksumState->launch_cost_limit = cost_limit;
+ LWLockRelease(DataChecksumsWorkerLock);
+ PG_RETURN_VOID();
+ }
+ LWLockRelease(DataChecksumsWorkerLock);
+
/*
* An invalid database cannot be connected to, so the worker would fail to
* process it, and unlike a dropped database its files stay around. Error
@@ -1254,6 +1273,7 @@ again:
pgstat_progress_update_param(PROGRESS_DATACHECKSUMS_PHASE,
PROGRESS_DATACHECKSUMS_PHASE_DISABLING);
+ INJECTION_POINT("datachecksums-disable-checksums-delay", NULL);
SetDataChecksumsOff();
ereport(LOG,
errmsg("data checksums are now disabled"));
diff --git a/src/test/modules/test_checksums/t/001_basic.pl b/src/test/modules/test_checksums/t/001_basic.pl
index 72e0d0df46f..62328c4fb36 100644
--- a/src/test/modules/test_checksums/t/001_basic.pl
+++ b/src/test/modules/test_checksums/t/001_basic.pl
@@ -61,8 +61,6 @@ is($result, '10000', 'ensure checksummed pages can be read back');
# Enabling checksums in a cluster which contains an invalid database left
# behind by an interrupted DROP DATABASE must be refused.
-disable_data_checksums($node, wait => 1);
-
$node->safe_psql('postgres', "CREATE DATABASE baddb;");
$node->safe_psql('baddb',
"CREATE TABLE bad_t AS SELECT generate_series(1,100) AS a;");
@@ -71,9 +69,19 @@ $node->safe_psql('baddb',
$node->safe_psql('postgres',
"UPDATE pg_database SET datconnlimit = -2 WHERE datname = 'baddb';");
+# A repeated enable is still a no-op when checksums are already on.
+my ($ret, $stdout, $stderr) =
+ $node->psql('postgres', "SELECT pg_enable_data_checksums();");
+is(
+ $ret, 0,
+ 'pg_enable_data_checksums is a no-op with an invalid database');
+test_checksum_state($node, 'on');
+
+disable_data_checksums($node, wait => 1);
+
# The request must fail up front with an actionable error, rather than fail
# halfway through processing.
-my ($ret, $stdout, $stderr) =
+($ret, $stdout, $stderr) =
$node->psql('postgres', "SELECT pg_enable_data_checksums();");
isnt($ret, 0, 'pg_enable_data_checksums fails with an invalid database');
like(
diff --git a/src/test/modules/test_checksums/t/005_injection.pl b/src/test/modules/test_checksums/t/005_injection.pl
index 34cd47e6c81..76b57245b9c 100644
--- a/src/test/modules/test_checksums/t/005_injection.pl
+++ b/src/test/modules/test_checksums/t/005_injection.pl
@@ -34,6 +34,54 @@ $node->start;
$node->safe_psql('postgres', 'CREATE EXTENSION test_checksums;');
$node->safe_psql('postgres', 'CREATE EXTENSION injection_points;');
+# ---------------------------------------------------------------------------
+# Test an enable request while disabling is already running
+#
+
+enable_data_checksums($node, wait => 'on');
+$node->safe_psql('postgres', 'CREATE DATABASE invalid_during_disable;');
+$node->safe_psql(
+ 'postgres',
+ "UPDATE pg_database SET datconnlimit = -2 "
+ . "WHERE datname = 'invalid_during_disable';");
+
+$node->safe_psql(
+ 'postgres',
+ "SELECT injection_points_attach("
+ . "'datachecksums-disable-checksums-delay', 'wait');");
+
+disable_data_checksums($node);
+$node->wait_for_event(
+ 'datachecksums launcher',
+ 'datachecksums-disable-checksums-delay');
+test_checksum_state($node, 'on');
+
+my ($enable_ret, $enable_stdout, $enable_stderr) =
+ $node->psql('postgres', 'SELECT pg_enable_data_checksums();');
+isnt(
+ $enable_ret, 0,
+ 'enable rejects invalid databases while disable is running');
+like(
+ $enable_stderr,
+ qr/invalid database "invalid_during_disable"/,
+ 'enable reports the invalid database while disable is running');
+
+$node->safe_psql(
+ 'postgres',
+ "SELECT injection_points_wakeup("
+ . "'datachecksums-disable-checksums-delay');");
+$node->safe_psql(
+ 'postgres',
+ "SELECT injection_points_detach("
+ . "'datachecksums-disable-checksums-delay');");
+
+$node->poll_query_until(
+ 'postgres',
+ "SELECT count(*) = 0 FROM pg_catalog.pg_stat_activity "
+ . "WHERE backend_type = 'datachecksums launcher';");
+test_checksum_state($node, 'off');
+$node->safe_psql('postgres', 'DROP DATABASE invalid_during_disable;');
+
# ---------------------------------------------------------------------------
# Inducing failures and crashes in processing
--
2.52.0
