From: Daniel Gustafsson Date: Tue, 28 Jul 2026 19:52:24 +0000 (+0200) Subject: Recheck checksum state before file_copy during CREATE DATABASE X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=153ca22a3a71ccaa55f27a4686202bce8f8de759;p=thirdparty%2Fpostgresql.git Recheck checksum state before file_copy during CREATE DATABASE The file_copy strategy check in createdb() runs during option validation, before the transaction has an XID and before the pg_database row exists, so the datachecksumsworker launcher can start in that window and see neither the new database nor the transaction creating it. It then raw-copies a template that was not processed yet, and those files stay unchecksummed, failing verification from then on. Recheck the state in CreateDatabaseUsingFileCopy(): the XID is assigned by then, so a launcher starting after this point waits for the transaction and finds the new database, and the copy errors out instead. Add an injection point before the catalog insert to test the window. Backpatch to v19 where online checksums were introduced. Author: Zsolt Parragi Reviewed-by: Daniel Gustafsson Discussion: https://postgr.es/m/CAN4CZFPEBsz8JeY4ixQ1V4ZL_xOY6pJaZS8ZLGH7R+wF--pEtg@mail.gmail.com Backpatch-through: 19 --- diff --git a/src/backend/commands/dbcommands.c b/src/backend/commands/dbcommands.c index 51dcbd9cace..e9766bcd7ef 100644 --- a/src/backend/commands/dbcommands.c +++ b/src/backend/commands/dbcommands.c @@ -65,6 +65,7 @@ #include "utils/acl.h" #include "utils/builtins.h" #include "utils/fmgroids.h" +#include "utils/injection_point.h" #include "utils/lsyscache.h" #include "utils/pg_locale.h" #include "utils/relmapper.h" @@ -558,6 +559,23 @@ CreateDatabaseUsingFileCopy(Oid src_dboid, Oid dst_dboid, Oid src_tsid, Relation rel; HeapTuple tuple; + /* + * The strategy check in createdb() runs before our transaction has an XID + * and before the pg_database row exists, so the datachecksumsworker + * launcher can start in that window and miss both the new database and + * our transaction, leaving the raw-copied files without checksums. + */ + if (DataChecksumsInProgressOn()) + ereport(ERROR, + errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), + errmsg("create database strategy \"%s\" not allowed when data checksums are being enabled", + "file_copy")); + + /* + * The XID is assigned by now, so a datachecksumsworker launcher starting + * after this point will wait for us and find the new database. + */ + /* * Force a checkpoint before starting the copy. This will force all dirty * buffers, including those of unlogged tables, out to disk, to ensure @@ -1045,6 +1063,14 @@ createdb(ParseState *pstate, const CreatedbStmt *stmt) dbstrategy = CREATEDB_WAL_LOG; else if (pg_strcasecmp(strategy, "file_copy") == 0) { + /* + * If data checksums are being enabled we must not use file_copy + * since it might copy source database which hasn't yet had data + * checksums enabled, and the destination database will be skipped + * as it's expected to have data checksums enabled. Once we have + * an XID assigned this needs to be rechecked, but if can error + * out already we can save a lot of work. + */ if (DataChecksumsInProgressOn()) ereport(ERROR, errcode(ERRCODE_INVALID_PARAMETER_VALUE), @@ -1510,6 +1536,8 @@ createdb(ParseState *pstate, const CreatedbStmt *stmt) tuple = heap_form_tuple(RelationGetDescr(pg_database_rel), new_record, new_record_nulls); + INJECTION_POINT("createdb-before-catalog-insert", NULL); + CatalogTupleInsert(pg_database_rel, tuple); /* diff --git a/src/test/modules/test_checksums/t/005_injection.pl b/src/test/modules/test_checksums/t/005_injection.pl index 7240b93bdd1..34cd47e6c81 100644 --- a/src/test/modules/test_checksums/t/005_injection.pl +++ b/src/test/modules/test_checksums/t/005_injection.pl @@ -76,5 +76,73 @@ SKIP: enable_data_checksums($node, wait => 'on'); } +# --------------------------------------------------------------------------- +# Test concurrent CREATE DATABASE which use the file_copy strategy +# + +disable_data_checksums($node, wait => 1); +my $node_loglocation = -s $node->logfile; + +$node->safe_psql('postgres', + "CREATE TABLE t AS SELECT generate_series(1,10000) AS a;"); + +$node->safe_psql('postgres', + "SELECT injection_points_attach('createdb-before-catalog-insert','wait');" +); +$node->safe_psql('postgres', + "SELECT injection_points_attach('datachecksumsworker-fake-temptable-wait','wait');" +); + +# Hold CREATE DATABASE after the strategy check, before its xact is visible. +my $bg = $node->background_psql('postgres'); +$bg->query_until( + qr/starting_create/, q( +\echo starting_create +CREATE DATABASE fcdb TEMPLATE template0 STRATEGY file_copy; +)); +$node->wait_for_event('client backend', 'createdb-before-catalog-insert'); + +# Enable checksums, worker holds before processing template0. +enable_data_checksums($node); +$node->wait_for_event('datachecksums worker', + 'datachecksumsworker-fake-temptable-wait'); + +# Release CREATE DATABASE, must fail on the recheck instead of raw-copying. +$node->safe_psql('postgres', + "SELECT injection_points_wakeup('createdb-before-catalog-insert');"); +$node->safe_psql('postgres', + "SELECT injection_points_detach('createdb-before-catalog-insert');"); + +# Wait for the CREATE DATABASE xact to finish before releasing the worker. +$node->poll_query_until('postgres', + "SELECT count(*) = 0 FROM pg_catalog.pg_stat_activity " + . "WHERE query LIKE 'CREATE DATABASE%' AND state != 'idle';"); + +$node->safe_psql('postgres', + "SELECT injection_points_wakeup('datachecksumsworker-fake-temptable-wait');" +); +$node->safe_psql('postgres', + "SELECT injection_points_detach('datachecksumsworker-fake-temptable-wait');" +); + +wait_for_checksum_state($node, 'on'); + +my $result = $node->safe_psql('postgres', + "SELECT count(*) FROM pg_catalog.pg_database WHERE datname = 'fcdb';"); +is($result, '0', 'file_copy database creation was refused'); + +my $log = + PostgreSQL::Test::Utils::slurp_file($node->logfile, $node_loglocation); +like( + $log, + qr/create database strategy "file_copy" not allowed/m, + 'file_copy error message in log'); + +# --------------------------------------------------------------------------- +# Test teardown +# + +$bg->{run}->finish; +$bg->quit; $node->stop; done_testing();