From 7943ac85ad8c124027bbc2921d35c771cba38d0c Mon Sep 17 00:00:00 2001 From: Zsolt Parragi Date: Mon, 20 Jul 2026 20:20:40 +0000 Subject: [PATCH] datachecksums: handle invalid and dropped databases during enable Enable errors out early with a hint when an invalid database exists, since the worker cannot connect to it and its files stay on disk. A worker that started but failed gets the same dropped-database heuristic as one that failed to start, so a concurrent drop during processing no longer aborts the whole run. The existence check locks the database first, otherwise a DROP DATABASE ... WITH (FORCE) which killed the worker is still only halfway done and the database looks like it is there to stay. --- src/backend/postmaster/datachecksum_state.c | 70 +++++++++++ .../modules/test_checksums/t/001_basic.pl | 117 ++++++++++++++++++ 2 files changed, 187 insertions(+) diff --git a/src/backend/postmaster/datachecksum_state.c b/src/backend/postmaster/datachecksum_state.c index 73dc539836b..ce13c59fde2 100644 --- a/src/backend/postmaster/datachecksum_state.c +++ b/src/backend/postmaster/datachecksum_state.c @@ -385,6 +385,7 @@ static void StartDataChecksumsWorkerLauncher(DataChecksumsWorkerOperation op, int cost_limit); static void DataChecksumsShmemRequest(void *arg); static bool DatabaseExists(Oid dboid); +static void ErrorOnInvalidDatabases(void); static List *BuildDatabaseList(void); static List *BuildRelationList(bool temp_relations, bool include_shared); static void FreeDatabaseList(List *dblist); @@ -583,6 +584,15 @@ enable_data_checksums(PG_FUNCTION_ARGS) errcode(ERRCODE_INVALID_PARAMETER_VALUE), errmsg("cost limit must be greater than zero")); + /* + * 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 + * out early with a hint rather than failing halfway through processing. + * A database which turns invalid after this check is handled by the + * launcher treating it as concurrently dropped. + */ + ErrorOnInvalidDatabases(); + StartDataChecksumsWorkerLauncher(ENABLE_DATACHECKSUMS, cost_delay, cost_limit); PG_RETURN_VOID(); @@ -984,6 +994,16 @@ ProcessDatabase(DataChecksumsWorkerDatabase *db) DataChecksumState->worker_pid = InvalidPid; LWLockRelease(DataChecksumsWorkerLock); + /* + * A worker which started but failed before reporting a result has most + * likely FATALed in InitPostgres. If the database was dropped, or was + * invalidated by a DROP DATABASE which is bound to remove its files, after + * we built the database list then that is the expected outcome and not an + * error, so apply the same heuristic as when the worker failed to start. + */ + if (result == DATACHECKSUMSWORKER_FAILED && !DatabaseExists(db->dboid)) + result = DATACHECKSUMSWORKER_DROPDB; + if (result == DATACHECKSUMSWORKER_ABORTED) ereport(LOG, errmsg("data checksums processing was aborted in database \"%s\"", @@ -1410,6 +1430,15 @@ DatabaseExists(Oid dboid) StartTransactionCommand(); + /* + * DROP DATABASE holds an exclusive lock on the database from before it + * terminates the connections to it until it commits, so take a lock which + * conflicts with it to wait out a drop which is in flight. Without this + * we can see a database whose worker was just killed by DROP DATABASE ... + * WITH (FORCE) as still existing, and report a spurious failure. + */ + LockSharedObject(DatabaseRelationId, dboid, 0, AccessShareLock); + rel = table_open(DatabaseRelationId, AccessShareLock); ScanKeyInit(&skey, Anum_pg_database_oid, @@ -1436,6 +1465,47 @@ DatabaseExists(Oid dboid) return found; } +/* + * ErrorOnInvalidDatabases + * Error out if the cluster contains an invalid database + * + * A database left invalid by an interrupted DROP DATABASE cannot be connected + * to, so data checksums can never be enabled in it, while its files remain on + * disk where checksum verification will find them. Report it to the caller + * so the user can drop it before retrying. Called from a normal backend, so + * unlike DatabaseExists we are already in a transaction. + * + * A cluster can contain more than one invalid database, but only the first one + * found is reported; collecting them all is not worth the complexity here. A + * user with several of them gets the error again for the next one after + * dropping the reported database, which the hint accounts for. + */ +static void +ErrorOnInvalidDatabases(void) +{ + Relation rel; + TableScanDesc scan; + HeapTuple tup; + + rel = table_open(DatabaseRelationId, AccessShareLock); + scan = table_beginscan_catalog(rel, 0, NULL); + + while (HeapTupleIsValid(tup = heap_getnext(scan, ForwardScanDirection))) + { + Form_pg_database pgdb = (Form_pg_database) GETSTRUCT(tup); + + if (database_is_invalid_form(pgdb)) + ereport(ERROR, + errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), + errmsg("cannot enable data checksums in a cluster with invalid database \"%s\"", + NameStr(pgdb->datname)), + errhint("Use DROP DATABASE to drop invalid databases.")); + } + + table_endscan(scan); + table_close(rel, AccessShareLock); +} + /* * BuildDatabaseList * Compile a list of all currently available databases in the cluster diff --git a/src/test/modules/test_checksums/t/001_basic.pl b/src/test/modules/test_checksums/t/001_basic.pl index a78118320d5..60ee252d536 100644 --- a/src/test/modules/test_checksums/t/001_basic.pl +++ b/src/test/modules/test_checksums/t/001_basic.pl @@ -59,5 +59,122 @@ enable_data_checksums($node, wait => 'on'); $result = $node->safe_psql('postgres', "SELECT count(*) FROM t WHERE a > 1"); 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;"); + +# Mark the database invalid, as an interrupted DROP DATABASE would. +$node->safe_psql('postgres', + "UPDATE pg_database SET datconnlimit = -2 WHERE datname = 'baddb';"); + +# The request must fail up front with an actionable error, rather than fail +# halfway through processing. +my ($ret, $stdout, $stderr) = + $node->psql('postgres', "SELECT pg_enable_data_checksums();"); +isnt($ret, 0, 'pg_enable_data_checksums fails with an invalid database'); +like( + $stderr, + qr/invalid database "baddb"/, + 'error message names the invalid database'); +like($stderr, qr/DROP DATABASE/, + 'error message hints at dropping the database'); +test_checksum_state($node, 'off'); + +# Dropping the invalid database clears the way. +$node->safe_psql('postgres', "DROP DATABASE baddb;"); +enable_data_checksums($node, wait => 'on'); + +# A database dropped while processing is in progress is not an error, the +# remaining databases are still processed. +disable_data_checksums($node, wait => 1); + +$node->safe_psql('postgres', "CREATE DATABASE dropme;"); +$node->safe_psql('dropme', + "CREATE TABLE dropme_t AS SELECT generate_series(1,10000) AS a;"); + +# Hold the worker in the "postgres" database by keeping a temporary table +# around, the worker waits for pre-existing temp tables to disappear before +# it reports the database as processed. "dropme" was created last, so it is +# processed after "postgres" and is still untouched while we wait. +my $bg = $node->background_psql('postgres'); +$bg->query_safe('CREATE TEMP TABLE holdme (a int);'); + +enable_data_checksums($node); + +$node->poll_query_until( + 'postgres', qq[ + SELECT count(*) > 0 FROM pg_stat_activity + WHERE backend_type = 'datachecksums worker' AND datname = 'postgres' + AND query LIKE 'Waiting for % temp tables to be removed'] +) or die "timed out waiting for worker to wait for temporary tables"; + +# Verify the assumption that processing has not reached "dropme" yet, without +# it the test would silently stop covering the concurrent drop. +my $log = slurp_file($node->logfile); +unlike( + $log, + qr/initiating data checksum processing in database "dropme"/, + 'processing has not reached the database to drop'); + +# Not processed yet and nobody is connected to it, so this must succeed. +$node->safe_psql('postgres', "DROP DATABASE dropme;"); + +# Let the worker in "postgres" finish, the launcher then moves on to the +# database which no longer exists. +$bg->query_safe('DROP TABLE holdme;'); +$bg->quit; + +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';"); + +# Same thing with DROP DATABASE ... WITH (FORCE), which terminates the +# checksums worker connected to the database being dropped. +disable_data_checksums($node, wait => 1); + +$node->safe_psql('postgres', "CREATE DATABASE dropmeforce;"); +$node->safe_psql('dropmeforce', + "CREATE TABLE dropme_t AS SELECT generate_series(1,10000) AS a;"); + +# Hold the worker inside "dropmeforce" by keeping a temporary table around +# there. +$bg = $node->background_psql('dropmeforce'); +$bg->query_safe('CREATE TEMP TABLE holdme (a int);'); + +enable_data_checksums($node); + +$node->poll_query_until( + 'postgres', qq[ + SELECT count(*) > 0 FROM pg_stat_activity + WHERE backend_type = 'datachecksums worker' AND datname = 'dropmeforce' + AND query LIKE 'Waiting for % temp tables to be removed'] +) or die "timed out waiting for worker to wait for temporary tables"; + +# Terminates both the session holding the temp table and the checksums +# worker connected to the database. +$node->safe_psql('postgres', "DROP DATABASE dropmeforce WITH (FORCE);"); +$bg->quit; + +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';"); + +$result = $node->safe_psql('postgres', "SELECT count(*) FROM t WHERE a > 1"); +is($result, '10000', 'ensure checksummed pages can be read back'); + $node->stop; + +# The resulting cluster must also pass offline verification, proving no +# unchecksummed files were left behind. +command_ok([ 'pg_checksums', '--check', '-D', $node->data_dir ], + 'offline checksum verification passes after enable'); + done_testing(); -- 2.54.0