Find invalid databases during upgrade check stage

Before continuing with the check start by checking that all databases
allow connections to avoid a hard fail without proper error reporting.

Inspired by a larger patch by Thomas Krennwallner.

Discussion: https://postgr.es/m/f9315bf0-e03e-4490-9f0d-5b6f7a6d9908@postsubmeta.net
This commit is contained in:
Daniel Gustafsson 2024-11-06 15:40:52 +01:00
parent 1e37cc6e2c
commit f638aafd1e
2 changed files with 24 additions and 15 deletions

View File

@ -16,7 +16,7 @@
static void check_new_cluster_is_empty(void); static void check_new_cluster_is_empty(void);
static void check_is_install_user(ClusterInfo *cluster); static void check_is_install_user(ClusterInfo *cluster);
static void check_proper_datallowconn(ClusterInfo *cluster); static void check_for_connection_status(ClusterInfo *cluster);
static void check_for_prepared_transactions(ClusterInfo *cluster); static void check_for_prepared_transactions(ClusterInfo *cluster);
static void check_for_isn_and_int8_passing_mismatch(ClusterInfo *cluster); static void check_for_isn_and_int8_passing_mismatch(ClusterInfo *cluster);
static void check_for_user_defined_postfix_ops(ClusterInfo *cluster); static void check_for_user_defined_postfix_ops(ClusterInfo *cluster);
@ -590,6 +590,12 @@ check_and_dump_old_cluster(void)
if (!user_opts.live_check) if (!user_opts.live_check)
start_postmaster(&old_cluster, true); start_postmaster(&old_cluster, true);
/*
* First check that all databases allow connections since we'll otherwise
* fail in later stages.
*/
check_for_connection_status(&old_cluster);
/* /*
* Extract a list of databases, tables, and logical replication slots from * Extract a list of databases, tables, and logical replication slots from
* the old cluster. * the old cluster.
@ -605,7 +611,6 @@ check_and_dump_old_cluster(void)
* Check for various failure cases * Check for various failure cases
*/ */
check_is_install_user(&old_cluster); check_is_install_user(&old_cluster);
check_proper_datallowconn(&old_cluster);
check_for_prepared_transactions(&old_cluster); check_for_prepared_transactions(&old_cluster);
check_for_isn_and_int8_passing_mismatch(&old_cluster); check_for_isn_and_int8_passing_mismatch(&old_cluster);
@ -1087,14 +1092,14 @@ check_is_install_user(ClusterInfo *cluster)
/* /*
* check_proper_datallowconn * check_for_connection_status
* *
* Ensure that all non-template0 databases allow connections since they * Ensure that all non-template0 databases allow connections since they
* otherwise won't be restored; and that template0 explicitly doesn't allow * 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.
*/ */
static void static void
check_proper_datallowconn(ClusterInfo *cluster) check_for_connection_status(ClusterInfo *cluster)
{ {
int dbnum; int dbnum;
PGconn *conn_template1; PGconn *conn_template1;
@ -1102,6 +1107,7 @@ check_proper_datallowconn(ClusterInfo *cluster)
int ntups; int ntups;
int i_datname; int i_datname;
int i_datallowconn; int i_datallowconn;
int i_datconnlimit;
FILE *script = NULL; FILE *script = NULL;
char output_path[MAXPGPATH]; char output_path[MAXPGPATH];
@ -1109,23 +1115,25 @@ check_proper_datallowconn(ClusterInfo *cluster)
snprintf(output_path, sizeof(output_path), "%s/%s", snprintf(output_path, sizeof(output_path), "%s/%s",
log_opts.basedir, log_opts.basedir,
"databases_with_datallowconn_false.txt"); "databases_cannot_connect_to.txt");
conn_template1 = connectToServer(cluster, "template1"); conn_template1 = connectToServer(cluster, "template1");
/* get database names */ /* get database names */
dbres = executeQueryOrDie(conn_template1, dbres = executeQueryOrDie(conn_template1,
"SELECT datname, datallowconn " "SELECT datname, datallowconn, datconnlimit "
"FROM pg_catalog.pg_database"); "FROM pg_catalog.pg_database");
i_datname = PQfnumber(dbres, "datname"); i_datname = PQfnumber(dbres, "datname");
i_datallowconn = PQfnumber(dbres, "datallowconn"); i_datallowconn = PQfnumber(dbres, "datallowconn");
i_datconnlimit = PQfnumber(dbres, "datconnlimit");
ntups = PQntuples(dbres); ntups = PQntuples(dbres);
for (dbnum = 0; dbnum < ntups; dbnum++) for (dbnum = 0; dbnum < ntups; dbnum++)
{ {
char *datname = PQgetvalue(dbres, dbnum, i_datname); char *datname = PQgetvalue(dbres, dbnum, i_datname);
char *datallowconn = PQgetvalue(dbres, dbnum, i_datallowconn); char *datallowconn = PQgetvalue(dbres, dbnum, i_datallowconn);
char *datconnlimit = PQgetvalue(dbres, dbnum, i_datconnlimit);
if (strcmp(datname, "template0") == 0) if (strcmp(datname, "template0") == 0)
{ {
@ -1137,10 +1145,11 @@ check_proper_datallowconn(ClusterInfo *cluster)
else else
{ {
/* /*
* avoid datallowconn == false databases from being skipped on * Avoid datallowconn == false databases from being skipped on
* restore * restore, and ensure that no databases are marked invalid with
* datconnlimit == -2.
*/ */
if (strcmp(datallowconn, "f") == 0) if ((strcmp(datallowconn, "f") == 0) || strcmp(datconnlimit, "-2") == 0)
{ {
if (script == NULL && (script = fopen_priv(output_path, "w")) == NULL) if (script == NULL && (script = fopen_priv(output_path, "w")) == NULL)
pg_fatal("could not open file \"%s\": %m", output_path); pg_fatal("could not open file \"%s\": %m", output_path);
@ -1159,11 +1168,11 @@ check_proper_datallowconn(ClusterInfo *cluster)
fclose(script); fclose(script);
pg_log(PG_REPORT, "fatal"); pg_log(PG_REPORT, "fatal");
pg_fatal("All non-template0 databases must allow connections, i.e. their\n" pg_fatal("All non-template0 databases must allow connections, i.e. their\n"
"pg_database.datallowconn must be true. Your installation contains\n" "pg_database.datallowconn must be true and pg_database.datconnlimit\n"
"non-template0 databases with their pg_database.datallowconn set to\n" "must not be -2. Your installation contains non-template0 databases\n"
"false. Consider allowing connection for all non-template0 databases\n" "which cannot be connected to. Consider allowing connection for all\n"
"or drop the databases which do not allow connections. A list of\n" "non-template0 databases or drop the databases which do not allow\n"
"databases with the problem is in the file:\n" "connections. A list of databases with the problem is in the file:\n"
" %s", output_path); " %s", output_path);
} }
else else

View File

@ -424,7 +424,7 @@ SKIP:
$mode, '--check', $mode, '--check',
], ],
1, 1,
[qr/invalid/], # pg_upgrade prints errors on stdout :( [qr/datconnlimit/],
[qr/^$/], [qr/^$/],
'invalid database causes failure'); 'invalid database causes failure');
rmtree($newnode->data_dir . "/pg_upgrade_output.d"); rmtree($newnode->data_dir . "/pg_upgrade_output.d");