From 6341415824448b51d1fd9b22ba0bc6105a22ac14 Mon Sep 17 00:00:00 2001 From: Bharath Rupireddy Date: Sat, 29 Aug 2026 12:44:24 +0000 Subject: [PATCH v1] Teach pg_upgrade to deal with invalid databases. Previously, pg_upgrade failed when it encountered an invalid database (commit c66a7d75e65, a database whose DROP DATABASE got interrupted) in the source cluster, blocking the whole upgrade workflow. Commit f638aafd1ea tightened checking and error reporting for invalid databases. This commit adds an --invalid-databases option, with the default being to skip. With skip, invalid databases are reported and left out of the upgrade; error retains the previous behavior of failing. Skipping is reasonable because if a user decided to DROP a database, the data in it is no longer wanted. Even if it were, the data in the files is unlikely to be consistent or useful, since DROP was interrupted mid-operation (while evicting buffer pool pages, unlinking files, etc.). Dropping invalid databases during upgrade could be costly, especially with large buffer pools and a large number of files to unlink. Skipping them instead is simpler, and the old directory contents are cleaned up by the removal script that pg_upgrade already generates. In the future, this could be extended to actively drop invalid databases during the upgrade. Author: Bharath Rupireddy Discussion: https://postgr.es/m/ --- doc/src/sgml/ref/pgupgrade.sgml | 24 ++++++++++++++ src/bin/pg_upgrade/check.c | 46 ++++++++++++++++++-------- src/bin/pg_upgrade/info.c | 2 ++ src/bin/pg_upgrade/option.c | 13 ++++++++ src/bin/pg_upgrade/pg_upgrade.h | 10 ++++++ src/bin/pg_upgrade/t/002_pg_upgrade.pl | 35 ++++++++++++++++---- src/tools/pgindent/typedefs.list | 1 + 7 files changed, 112 insertions(+), 19 deletions(-) diff --git a/doc/src/sgml/ref/pgupgrade.sgml b/doc/src/sgml/ref/pgupgrade.sgml index e4e8c02e6d6..8cd9f13b6dd 100644 --- a/doc/src/sgml/ref/pgupgrade.sgml +++ b/doc/src/sgml/ref/pgupgrade.sgml @@ -262,6 +262,30 @@ PostgreSQL documentation + + option + + + Choose how to treat invalid databases in the old cluster, that is + databases whose DROP DATABASE was interrupted, + leaving them with datconnlimit set to + -2 in + pg_database. + Such a database can no longer be connected to and can never be + upgraded. Possible values are skip (the default) + and error. + + + With skip, invalid databases are reported and left + behind while the rest of the cluster is upgraded; their files are + removed along with the old cluster by the generated + delete_old_cluster script. With + error, invalid databases are reported and the + upgrade is aborted. + + + + diff --git a/src/bin/pg_upgrade/check.c b/src/bin/pg_upgrade/check.c index 41342561763..ef9e34f1e7e 100644 --- a/src/bin/pg_upgrade/check.c +++ b/src/bin/pg_upgrade/check.c @@ -1068,7 +1068,10 @@ 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. + * connections since it would make pg_dumpall --globals restore fail. Invalid + * databases (whose DROP DATABASE was interrupted) are skipped by default and + * reported here; with --invalid-databases=error they abort the upgrade + * instead. */ static void check_for_connection_status(ClusterInfo *cluster) @@ -1081,6 +1084,7 @@ check_for_connection_status(ClusterInfo *cluster) int i_datallowconn; int i_datconnlimit; FILE *script = NULL; + bool skipped_invalid = false; char output_path[MAXPGPATH]; prep_status("Checking database connection settings"); @@ -1114,20 +1118,36 @@ check_for_connection_status(ClusterInfo *cluster) pg_fatal("template0 must not allow connections, " "i.e. its pg_database.datallowconn must be false"); } - else + + /* + * Skip invalid databases unless the user asked us to treat them as an + * error. They are left out of the upgrade, and their files are + * removed along with the rest of the old cluster by the generated + * delete_old_cluster script. + */ + else if (strcmp(datconnlimit, "-2") == 0 && + user_opts.invalid_db_mode == INVALID_DB_SKIP) { - /* - * Avoid datallowconn == false databases from being skipped on - * restore, and ensure that no databases are marked invalid with - * datconnlimit == -2. - */ - if ((strcmp(datallowconn, "f") == 0) || strcmp(datconnlimit, "-2") == 0) + if (!skipped_invalid) { - 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); + report_status(PG_WARNING, "warning"); + skipped_invalid = true; } + pg_log(PG_WARNING, + "invalid database \"%s\" will not be upgraded", datname); + } + + /* + * Databases that disallow connections would be silently skipped on + * restore, and invalid databases in --invalid-databases=error mode + * must not be carried over, so both block the upgrade. + */ + else 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); + + fprintf(script, "%s\n", datname); } } @@ -1147,7 +1167,7 @@ check_for_connection_status(ClusterInfo *cluster) "connections. A list of databases with the problem is in the file:\n" " %s", output_path); } - else + else if (!skipped_invalid) check_ok(); } diff --git a/src/bin/pg_upgrade/info.c b/src/bin/pg_upgrade/info.c index 37fff93892f..09f0b739f26 100644 --- a/src/bin/pg_upgrade/info.c +++ b/src/bin/pg_upgrade/info.c @@ -423,6 +423,8 @@ get_db_infos(ClusterInfo *cluster) " LEFT OUTER JOIN pg_catalog.pg_tablespace t " " ON d.dattablespace = t.oid " "WHERE d.datallowconn = true " + /* skip invalid databases, whose DROP DATABASE was interrupted */ + " AND d.datconnlimit <> -2 " "ORDER BY 1"); res = executeQueryOrDie(conn, "%s", query); diff --git a/src/bin/pg_upgrade/option.c b/src/bin/pg_upgrade/option.c index f01d2f92d95..5ef08e23ffd 100644 --- a/src/bin/pg_upgrade/option.c +++ b/src/bin/pg_upgrade/option.c @@ -63,6 +63,7 @@ parseCommandLine(int argc, char *argv[]) {"no-statistics", no_argument, NULL, 5}, {"set-char-signedness", required_argument, NULL, 6}, {"swap", no_argument, NULL, 7}, + {"invalid-databases", required_argument, NULL, 8}, {NULL, 0, NULL, 0} }; @@ -75,6 +76,7 @@ parseCommandLine(int argc, char *argv[]) user_opts.transfer_mode = TRANSFER_MODE_COPY; user_opts.do_statistics = true; user_opts.char_signedness = -1; + user_opts.invalid_db_mode = INVALID_DB_SKIP; os_info.progname = get_progname(argv[0]); @@ -234,6 +236,15 @@ parseCommandLine(int argc, char *argv[]) user_opts.transfer_mode = TRANSFER_MODE_SWAP; break; + case 8: + if (pg_strcasecmp(optarg, "skip") == 0) + user_opts.invalid_db_mode = INVALID_DB_SKIP; + else if (pg_strcasecmp(optarg, "error") == 0) + user_opts.invalid_db_mode = INVALID_DB_ERROR; + else + pg_fatal("invalid argument for option %s", "--invalid-databases"); + break; + default: fprintf(stderr, _("Try \"%s --help\" for more information.\n"), os_info.progname); @@ -328,6 +339,8 @@ usage(void) printf(_(" --clone clone instead of copying files to new cluster\n")); printf(_(" --copy copy files to new cluster (default)\n")); printf(_(" --copy-file-range copy files to new cluster with copy_file_range\n")); + printf(_(" --invalid-databases=OPTION how to treat invalid databases, \"skip\"\n" + " (default) or \"error\"\n")); printf(_(" --no-statistics do not import statistics from old cluster\n")); printf(_(" --set-char-signedness=OPTION set new cluster char signedness to \"signed\" or\n" " \"unsigned\"\n")); diff --git a/src/bin/pg_upgrade/pg_upgrade.h b/src/bin/pg_upgrade/pg_upgrade.h index c80e8fb4031..b73bb4021ed 100644 --- a/src/bin/pg_upgrade/pg_upgrade.h +++ b/src/bin/pg_upgrade/pg_upgrade.h @@ -248,6 +248,15 @@ typedef enum TRANSFER_MODE_SWAP, } transferMode; +/* + * Enumeration to denote how to treat invalid databases in the old cluster + */ +typedef enum +{ + INVALID_DB_SKIP, /* skip them, don't upgrade them (default) */ + INVALID_DB_ERROR, /* report them and abort the upgrade */ +} invalidDbMode; + /* * Enumeration to denote pg_log modes */ @@ -325,6 +334,7 @@ typedef struct int char_signedness; /* default char signedness: -1 for initial * value, 1 for "signed" and 0 for * "unsigned" */ + invalidDbMode invalid_db_mode; /* how to treat invalid databases */ } UserOpts; typedef struct diff --git a/src/bin/pg_upgrade/t/002_pg_upgrade.pl b/src/bin/pg_upgrade/t/002_pg_upgrade.pl index 0a4121fdc4d..647a8ce5ce9 100644 --- a/src/bin/pg_upgrade/t/002_pg_upgrade.pl +++ b/src/bin/pg_upgrade/t/002_pg_upgrade.pl @@ -513,14 +513,17 @@ 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"); -# 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.) +# Check how pg_upgrade handles an invalid database. With +# --invalid-databases=error it is reported and the upgrade aborts; by default +# it is skipped and the upgrade proceeds. (Versions that were out of support +# by commit c66a7d75e652 can't mark a database invalid, so skip these checks +# there.) SKIP: { - skip "database invalidation not implemented", 1 + skip "database invalidation not implemented", 2 if $oldnode->pg_version < 11; + # --invalid-databases=error preserves the historical hard failure. command_checks_all( [ 'pg_upgrade', '--no-sync', @@ -531,12 +534,32 @@ SKIP: '--socketdir' => $newnode->host, '--old-port' => $oldnode->port, '--new-port' => $newnode->port, - $mode, '--check', + $mode, '--check', '--invalid-databases' => 'error', ], 1, [qr/datconnlimit/], [qr/^$/], - 'invalid database causes failure'); + 'invalid database causes failure with --invalid-databases=error'); + rmtree($newnode->data_dir . "/pg_upgrade_output.d"); + + # The default skips invalid databases, so --check succeeds while naming + # the database that will not be upgraded. + 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', + ], + 0, + [qr/invalid database "regression_invalid" will not be upgraded/], + [qr/^$/], + 'invalid database is skipped by default'); rmtree($newnode->data_dir . "/pg_upgrade_output.d"); } diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index 15b10e1703f..5177d340dfa 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -3913,6 +3913,7 @@ intset_internal_node intset_leaf_node intset_node intvKEY +invalidDbMode io_callback_fn io_stat_col itemIdCompact -- 2.47.3