Hi, On Tue, Oct 6, 2026 at 11:32 AM Robert Treat <[email protected]> wrote: > > > > Thanks. IMHO we shouldn't bother adding a note to pg_upgrade's > > > documentation, or even emitting warnings when pg_upgrade skips invalid > > > databases. > > > Upon thinking more on this, I agree on both. Unless there are > > objections, I will drop the warning and the doc note in the next > > version. > > > I also think not reporting them matches what other tools do, like > > pg_dumpall (dumpDatabases()), vacuumdb (vacuum_all_databases()), > > reindexdb (reindex_all_databases()), etc. They skip invalid databases > > without emitting any info, and their docs don't mention it either. > > I guess I don't object, but it feels a little odd to make a user > visible behavioral change without giving any kinds of heads up; this > would have errored out in previous versions, now it "just works", but > just works might look concerning if a database that used to show up in > your database list now goes missing, or you are wondering why your > cluster size dropped significantly after upgrade. There are some other > weird corner cases that might come up too,
Other tools silently ignoring the invalid databases worked fine so far (unless I'm missing anything here), so it shouldn't be a big problem IMHO. > but hopefully the release > note entry will be enough for people looking for explanations. Yes, this will help. Please find the attached v3 patch dropping the docs note and the warning. I self-reviewed it and fixed a couple of things: excluded invalid databases while checking encodings and global names on the old cluster, and reworded the commit message. -- Bharath Rupireddy Amazon Web Services: https://aws.amazon.com
From c3f8aeeca952891386c13257ebe00e673b5fff91 Mon Sep 17 00:00:00 2001 From: Bharath Rupireddy <[email protected]> Date: Thu, 15 Oct 2026 03:31:34 +0000 Subject: [PATCH v3] Teach pg_upgrade to skip invalid databases. Previously, pg_upgrade failed when it encountered an invalid database in the old cluster, that is, a database whose DROP DATABASE got interrupted. Commit f638aafd1ea8 taught it to recognize such databases and report them up front, instead of failing later when trying to connect to them. This blocks the upgrade until the user analyzes the failure, identifies the issue from the logs, starts the old cluster, issues DROP DATABASE on each such database, and restarts the upgrade, which is time-consuming and tedious. These databases are not connectable or usable, and there is no supported recovery mechanism for them. The user has already decided to drop them, and they need to be dropped eventually anyway. Even if the data were still wanted, it is unlikely to be consistent or useful, since DROP was interrupted partway through evicting buffer pool pages, unlinking files, etc. This commit teaches pg_upgrade to skip them instead of erroring out. They are not part of the schema dump or the file transfer, so nothing of theirs reaches the new cluster in any transfer mode. Their files stay in the old cluster, which remains usable as a fallback in copy and clone modes, so they can still be dropped there. Note that the skipped databases are not reported, since there is nothing the user needs to do about them. Author: Bharath Rupireddy <[email protected]> Reviewed-by: Nathan Bossart <[email protected]> Reviewed-by: Rithvika Devisetti <[email protected]> Reviewed-by: Robert Treat <[email protected]> Reviewed-by: Andres Freund <[email protected]> Reviewed-by: Euler Taveira <[email protected]> Discussion: https://postgr.es/m/CALj2ACV3jSPaMCSRQ8XZTN_meZezNyySpMJVhVBZy_qDJK2mmQ%40mail.gmail.com --- src/bin/pg_upgrade/check.c | 55 +++++++++++------ src/bin/pg_upgrade/info.c | 5 +- src/bin/pg_upgrade/t/002_pg_upgrade.pl | 84 +++++++++++++++----------- 3 files changed, 91 insertions(+), 53 deletions(-) diff --git a/src/bin/pg_upgrade/check.c b/src/bin/pg_upgrade/check.c index 150c4fc34b9..9fb012950bb 100644 --- a/src/bin/pg_upgrade/check.c +++ b/src/bin/pg_upgrade/check.c @@ -1107,6 +1107,9 @@ check_is_install_user(ClusterInfo *cluster) * Ensure that all non-template0 databases allow connections since they * otherwise won't be restored; and that template0 explicitly doesn't allow * connections since it would make pg_dumpall --globals restore fail. + * + * Invalid databases, that is, databases whose DROP DATABASE got interrupted, + * can't be connected to and hence can't be upgraded, so they are skipped. */ static void check_for_connection_status(ClusterInfo *cluster) @@ -1152,20 +1155,26 @@ check_for_connection_status(ClusterInfo *cluster) pg_fatal("template0 must not allow connections, " "i.e. its pg_database.datallowconn must be false"); } - else + else if (strcmp(datconnlimit, "-2") == 0) + { + /* + * Skip invalid databases, they can't be upgraded. Check this + * before datallowconn, since an invalid database can have + * datallowconn = false too, and allowing connections would still + * not make it upgradable. + */ + continue; + } + else if (strcmp(datallowconn, "f") == 0) { /* * Avoid datallowconn == false databases from being skipped on - * restore, and ensure that no databases are marked invalid with - * datconnlimit == -2. + * restore. */ - if ((strcmp(datallowconn, "f") == 0) || strcmp(datconnlimit, "-2") == 0) - { - if (script == NULL && (script = fopen_priv(output_path, "w")) == NULL) - pg_fatal("could not open file \"%s\": %m", output_path); + if (script == NULL && (script = fopen_priv(output_path, "w")) == NULL) + pg_fatal("could not open file \"%s\": %m", output_path); - fprintf(script, "%s\n", datname); - } + fprintf(script, "%s\n", datname); } } @@ -1178,11 +1187,11 @@ check_for_connection_status(ClusterInfo *cluster) fclose(script); pg_log(PG_REPORT, "fatal"); pg_fatal("All non-template0 databases must allow connections, i.e. their\n" - "pg_database.datallowconn must be true and pg_database.datconnlimit\n" - "must not be -2. Your installation contains non-template0 databases\n" - "which cannot be connected to. Consider allowing connection for all\n" - "non-template0 databases or drop the databases which do not allow\n" - "connections. A list of databases with the problem is in the file:\n" + "pg_database.datallowconn must be true. Your installation contains\n" + "non-template0 databases which cannot be connected to. Consider\n" + "allowing connection for all non-template0 databases or drop the\n" + "databases which do not allow connections. A list of databases with\n" + "the problem is in the file:\n" " %s", output_path); } else @@ -1212,9 +1221,11 @@ check_for_unsupported_encodings(ClusterInfo *cluster) conn = connectToServer(cluster, "template1"); + /* Invalid databases are skipped, so their encoding doesn't matter */ res = executeQueryOrDie(conn, "SELECT datname, encoding " - "FROM pg_catalog.pg_database"); + "FROM pg_catalog.pg_database " + "WHERE datconnlimit <> -2"); ntups = PQntuples(res); i_datname = PQfnumber(res, "datname"); i_encoding = PQfnumber(res, "encoding"); @@ -2566,11 +2577,13 @@ check_old_cluster_global_names(ClusterInfo *cluster) /* * Get database, user/role and tablespace names from cluster. Can't use - * pg_authid because only superusers can view it. + * pg_authid because only superusers can view it. Invalid databases are + * skipped, so their names don't matter. */ res = executeQueryOrDie(conn_template1, "SELECT datname AS objname, 'database' AS objtype " - "FROM pg_catalog.pg_database UNION ALL " + "FROM pg_catalog.pg_database " + "WHERE datconnlimit <> -2 UNION ALL " "SELECT rolname AS objname, 'role' AS objtype " "FROM pg_catalog.pg_roles UNION ALL " "SELECT spcname AS objname, 'tablespace' AS objtype " @@ -2643,9 +2656,15 @@ check_for_oldestxid_consistency(ClusterInfo *cluster) conn_template1 = connectToServer(cluster, "template1"); + /* + * Invalid databases are skipped, and VACUUM ignores them when it advances + * the cluster-wide oldest XID and multixact ID, so it is normal for their + * datfrozenxid and datminmxid to lag behind the control file. + */ dbres = executeQueryOrDie(conn_template1, "SELECT datname, datfrozenxid, datminmxid " - "FROM pg_catalog.pg_database"); + "FROM pg_catalog.pg_database " + "WHERE datconnlimit <> -2"); i_datname = PQfnumber(dbres, "datname"); i_datfrozenxid = PQfnumber(dbres, "datfrozenxid"); diff --git a/src/bin/pg_upgrade/info.c b/src/bin/pg_upgrade/info.c index 5c59cfb32e8..9417ffe69c7 100644 --- a/src/bin/pg_upgrade/info.c +++ b/src/bin/pg_upgrade/info.c @@ -406,6 +406,7 @@ get_db_infos(ClusterInfo *cluster) i_spclocation; char query[QUERY_ALLOC]; + /* Skip invalid databases in the old cluster */ snprintf(query, sizeof(query), "SELECT d.oid, d.datname, " "pg_catalog.pg_tablespace_location(t.oid) AS spclocation " @@ -413,7 +414,9 @@ get_db_infos(ClusterInfo *cluster) " LEFT OUTER JOIN pg_catalog.pg_tablespace t " " ON d.dattablespace = t.oid " "WHERE d.datallowconn = true " - "ORDER BY 1"); + "%s" + "ORDER BY 1", + cluster == &old_cluster ? "AND d.datconnlimit <> -2 " : ""); res = executeQueryOrDie(conn, "%s", query); diff --git a/src/bin/pg_upgrade/t/002_pg_upgrade.pl b/src/bin/pg_upgrade/t/002_pg_upgrade.pl index be57b2d838c..e3ecb1ab531 100644 --- a/src/bin/pg_upgrade/t/002_pg_upgrade.pl +++ b/src/bin/pg_upgrade/t/002_pg_upgrade.pl @@ -551,55 +551,59 @@ SKIP: 'run of pg_upgrade --check with old instance running'); } -# Create an invalid database, will be deleted below +# Create an invalid database, it must not be upgraded $oldnode->safe_psql( 'postgres', qq( CREATE DATABASE regression_invalid; UPDATE pg_database SET datconnlimit = -2 WHERE datname = 'regression_invalid'; )); +my $invalid_db_oid = $oldnode->safe_psql('postgres', + "SELECT oid FROM pg_database WHERE datname = 'regression_invalid'"); + +# Create a database that does not allow connections, which blocks the +# upgrade, and is dropped below so that the upgrade can proceed. +$oldnode->safe_psql( + 'postgres', qq( + CREATE DATABASE regression_noconn; + UPDATE pg_database SET datallowconn = false WHERE datname = 'regression_noconn'; +)); # Upgrade the instance. $oldnode->stop; -# Check that pg_upgrade aborts when encountering an invalid database -# (However, versions that were out of support by commit c66a7d75e652 don't -# know how to do this, so skip this test there.) -SKIP: -{ - skip "database invalidation not implemented", 4 - if $oldnode->pg_version < 11; - - command_checks_all( - [ - 'pg_upgrade', '--no-sync', - '--old-datadir' => $oldnode->data_dir, - '--new-datadir' => $newnode->data_dir, - '--old-bindir' => $oldbindir, - '--new-bindir' => $newbindir, - '--socketdir' => $newnode->host, - '--old-port' => $oldnode->port, - '--new-port' => $newnode->port, - $mode, '--check', - ], - 1, - [qr/datconnlimit/], - [qr/^$/], - 'invalid database causes failure'); +# Check that pg_upgrade aborts when a database does not allow connections. +# This also creates the logging directory pg_upgrade_output.d and leaves it +# around, as happens for any failure. +command_checks_all( + [ + 'pg_upgrade', '--no-sync', + '--old-datadir' => $oldnode->data_dir, + '--new-datadir' => $newnode->data_dir, + '--old-bindir' => $oldbindir, + '--new-bindir' => $newbindir, + '--socketdir' => $newnode->host, + '--old-port' => $oldnode->port, + '--new-port' => $newnode->port, + $mode, '--check', + ], + 1, + [qr/All non-template0 databases must allow connections/], + [qr/^$/], + 'database that does not allow connections causes failure'); - # Check that the failure creates the logging directory pg_upgrade_output.d - # and leaves it around - ok(-d $newnode->data_dir . "/pg_upgrade_output.d", - "pg_upgrade_output.d/ not removed after pg_upgrade failure"); +ok(-d $newnode->data_dir . "/pg_upgrade_output.d", + "pg_upgrade_output.d/ not removed after pg_upgrade failure"); - rmtree($newnode->data_dir . "/pg_upgrade_output.d"); -} +rmtree($newnode->data_dir . "/pg_upgrade_output.d"); -# And drop it, so we can continue +# And drop the database that does not allow connections, so we can continue $oldnode->start; -$oldnode->safe_psql('postgres', 'DROP DATABASE regression_invalid'); +$oldnode->safe_psql('postgres', 'DROP DATABASE regression_noconn'); $oldnode->stop; -# --check command works here, cleans up pg_upgrade_output.d. +# --check command works here, cleans up pg_upgrade_output.d. The invalid +# database is left in place, so this also checks that the upgrade is not +# aborted because of it. command_ok( [ 'pg_upgrade', '--no-sync', @@ -635,6 +639,18 @@ ok( !-d $newnode->data_dir . "/pg_upgrade_output.d", $newnode->start; +# The invalid database must not have made it into the new cluster. +is( $newnode->safe_psql( + 'postgres', + "SELECT count(*) FROM pg_database WHERE datname = 'regression_invalid'" + ), + '0', + 'invalid database is not present in the new cluster'); +ok(!-d $newnode->data_dir . "/base/$invalid_db_oid", + "invalid database is not transferred to the new cluster"); +ok(-d $oldnode->data_dir . "/base/$invalid_db_oid", + "invalid database is left behind in the old cluster"); + # The 8-byte OID has been carried. if (!defined($ENV{oldinstall})) { -- 2.47.3
