pg_upgrade: Move live_check variable to user_opts.

At the moment, pg_upgrade stores whether it is doing a "live check"
(i.e., the user specified --check and the old server is still
running) in a local variable scoped to main().  This live_check
variable is passed to several functions.  To further complicate
matters, a few call sites provide a hard-coded "false" as the
live_check argument.  Specifically, this is done when calling these
functions for the new cluster, for which any live-check-only paths
won't apply.

This commit moves the live_check variable to the global user_opts
variable, which stores information about the options the user
specified on the command line.  This allows us to remove the
live_check parameter from several functions.  For the functions
with callers that provide a hard-coded "false" as the live_check
argument (e.g., get_control_data()), we verify the given cluster is
the old cluster before taking any live-check-only paths.

This small refactoring effort helps simplify some proposed changes
that would parallelize many of pg_upgrade's once-in-each-database
tasks using libpq's asynchronous APIs.  By removing the live_check
parameter, we can more easily convert the functions to callbacks
for the new parallel system.

Reviewed-by: Daniel Gustafsson
Discussion: https://postgr.es/m/20240516211638.GA1688936%40nathanxps13
This commit is contained in:
Nathan Bossart 2024-07-26 13:37:32 -05:00
parent 5d1d8b3c82
commit 4b56bb4ab4
6 changed files with 41 additions and 42 deletions

View File

@ -29,7 +29,7 @@ static void check_for_new_tablespace_dir(void);
static void check_for_user_defined_encoding_conversions(ClusterInfo *cluster); static void check_for_user_defined_encoding_conversions(ClusterInfo *cluster);
static void check_new_cluster_logical_replication_slots(void); static void check_new_cluster_logical_replication_slots(void);
static void check_new_cluster_subscription_configuration(void); static void check_new_cluster_subscription_configuration(void);
static void check_old_cluster_for_valid_slots(bool live_check); static void check_old_cluster_for_valid_slots(void);
static void check_old_cluster_subscription_state(void); static void check_old_cluster_subscription_state(void);
/* /*
@ -555,9 +555,9 @@ fix_path_separator(char *path)
} }
void void
output_check_banner(bool live_check) output_check_banner(void)
{ {
if (user_opts.check && live_check) if (user_opts.live_check)
{ {
pg_log(PG_REPORT, pg_log(PG_REPORT,
"Performing Consistency Checks on Old Live Server\n" "Performing Consistency Checks on Old Live Server\n"
@ -573,18 +573,18 @@ output_check_banner(bool live_check)
void void
check_and_dump_old_cluster(bool live_check) check_and_dump_old_cluster(void)
{ {
/* -- OLD -- */ /* -- OLD -- */
if (!live_check) if (!user_opts.live_check)
start_postmaster(&old_cluster, true); start_postmaster(&old_cluster, true);
/* /*
* 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.
*/ */
get_db_rel_and_slot_infos(&old_cluster, live_check); get_db_rel_and_slot_infos(&old_cluster);
init_tablespaces(); init_tablespaces();
@ -605,7 +605,7 @@ check_and_dump_old_cluster(bool live_check)
* Logical replication slots can be migrated since PG17. See comments * Logical replication slots can be migrated since PG17. See comments
* atop get_old_cluster_logical_slot_infos(). * atop get_old_cluster_logical_slot_infos().
*/ */
check_old_cluster_for_valid_slots(live_check); check_old_cluster_for_valid_slots();
/* /*
* Subscriptions and their dependencies can be migrated since PG17. * Subscriptions and their dependencies can be migrated since PG17.
@ -669,7 +669,7 @@ check_and_dump_old_cluster(bool live_check)
if (!user_opts.check) if (!user_opts.check)
generate_old_dump(); generate_old_dump();
if (!live_check) if (!user_opts.live_check)
stop_postmaster(false); stop_postmaster(false);
} }
@ -677,7 +677,7 @@ check_and_dump_old_cluster(bool live_check)
void void
check_new_cluster(void) check_new_cluster(void)
{ {
get_db_rel_and_slot_infos(&new_cluster, false); get_db_rel_and_slot_infos(&new_cluster);
check_new_cluster_is_empty(); check_new_cluster_is_empty();
@ -828,14 +828,14 @@ check_cluster_versions(void)
void void
check_cluster_compatibility(bool live_check) check_cluster_compatibility(void)
{ {
/* get/check pg_control data of servers */ /* get/check pg_control data of servers */
get_control_data(&old_cluster, live_check); get_control_data(&old_cluster);
get_control_data(&new_cluster, false); get_control_data(&new_cluster);
check_control_data(&old_cluster.controldata, &new_cluster.controldata); check_control_data(&old_cluster.controldata, &new_cluster.controldata);
if (live_check && old_cluster.port == new_cluster.port) if (user_opts.live_check && old_cluster.port == new_cluster.port)
pg_fatal("When checking a live server, " pg_fatal("When checking a live server, "
"the old and new port numbers must be different."); "the old and new port numbers must be different.");
} }
@ -1838,7 +1838,7 @@ check_new_cluster_subscription_configuration(void)
* before shutdown. * before shutdown.
*/ */
static void static void
check_old_cluster_for_valid_slots(bool live_check) check_old_cluster_for_valid_slots(void)
{ {
char output_path[MAXPGPATH]; char output_path[MAXPGPATH];
FILE *script = NULL; FILE *script = NULL;
@ -1877,7 +1877,7 @@ check_old_cluster_for_valid_slots(bool live_check)
* Note: This can be satisfied only when the old cluster has been * Note: This can be satisfied only when the old cluster has been
* shut down, so we skip this for live checks. * shut down, so we skip this for live checks.
*/ */
if (!live_check && !slot->caught_up) if (!user_opts.live_check && !slot->caught_up)
{ {
if (script == NULL && if (script == NULL &&
(script = fopen_priv(output_path, "w")) == NULL) (script = fopen_priv(output_path, "w")) == NULL)

View File

@ -33,7 +33,7 @@
* return valid xid data for a running server. * return valid xid data for a running server.
*/ */
void void
get_control_data(ClusterInfo *cluster, bool live_check) get_control_data(ClusterInfo *cluster)
{ {
char cmd[MAXPGPATH]; char cmd[MAXPGPATH];
char bufin[MAX_STRING]; char bufin[MAX_STRING];
@ -76,6 +76,7 @@ get_control_data(ClusterInfo *cluster, bool live_check)
uint32 segno = 0; uint32 segno = 0;
char *resetwal_bin; char *resetwal_bin;
int rc; int rc;
bool live_check = (cluster == &old_cluster && user_opts.live_check);
/* /*
* Because we test the pg_resetwal output as strings, it has to be in * Because we test the pg_resetwal output as strings, it has to be in

View File

@ -27,7 +27,7 @@ static void free_rel_infos(RelInfoArr *rel_arr);
static void print_db_infos(DbInfoArr *db_arr); static void print_db_infos(DbInfoArr *db_arr);
static void print_rel_infos(RelInfoArr *rel_arr); static void print_rel_infos(RelInfoArr *rel_arr);
static void print_slot_infos(LogicalSlotInfoArr *slot_arr); static void print_slot_infos(LogicalSlotInfoArr *slot_arr);
static void get_old_cluster_logical_slot_infos(DbInfo *dbinfo, bool live_check); static void get_old_cluster_logical_slot_infos(DbInfo *dbinfo);
/* /*
@ -272,11 +272,9 @@ report_unmatched_relation(const RelInfo *rel, const DbInfo *db, bool is_new_db)
* *
* higher level routine to generate dbinfos for the database running * higher level routine to generate dbinfos for the database running
* on the given "port". Assumes that server is already running. * on the given "port". Assumes that server is already running.
*
* live_check would be used only when the target is the old cluster.
*/ */
void void
get_db_rel_and_slot_infos(ClusterInfo *cluster, bool live_check) get_db_rel_and_slot_infos(ClusterInfo *cluster)
{ {
int dbnum; int dbnum;
@ -293,7 +291,7 @@ get_db_rel_and_slot_infos(ClusterInfo *cluster, bool live_check)
get_rel_infos(cluster, pDbInfo); get_rel_infos(cluster, pDbInfo);
if (cluster == &old_cluster) if (cluster == &old_cluster)
get_old_cluster_logical_slot_infos(pDbInfo, live_check); get_old_cluster_logical_slot_infos(pDbInfo);
} }
if (cluster == &old_cluster) if (cluster == &old_cluster)
@ -637,7 +635,7 @@ get_rel_infos(ClusterInfo *cluster, DbInfo *dbinfo)
* are included. * are included.
*/ */
static void static void
get_old_cluster_logical_slot_infos(DbInfo *dbinfo, bool live_check) get_old_cluster_logical_slot_infos(DbInfo *dbinfo)
{ {
PGconn *conn; PGconn *conn;
PGresult *res; PGresult *res;
@ -673,7 +671,7 @@ get_old_cluster_logical_slot_infos(DbInfo *dbinfo, bool live_check)
"WHERE slot_type = 'logical' AND " "WHERE slot_type = 'logical' AND "
"database = current_database() AND " "database = current_database() AND "
"temporary IS FALSE;", "temporary IS FALSE;",
live_check ? "FALSE" : user_opts.live_check ? "FALSE" :
"(CASE WHEN invalidation_reason IS NOT NULL THEN FALSE " "(CASE WHEN invalidation_reason IS NOT NULL THEN FALSE "
"ELSE (SELECT pg_catalog.binary_upgrade_logical_slot_has_caught_up(slot_name)) " "ELSE (SELECT pg_catalog.binary_upgrade_logical_slot_has_caught_up(slot_name)) "
"END)"); "END)");

View File

@ -470,10 +470,10 @@ adjust_data_dir(ClusterInfo *cluster)
* directory. * directory.
*/ */
void void
get_sock_dir(ClusterInfo *cluster, bool live_check) get_sock_dir(ClusterInfo *cluster)
{ {
#if !defined(WIN32) #if !defined(WIN32)
if (!live_check) if (!user_opts.live_check || cluster == &new_cluster)
cluster->sockdir = user_opts.socketdir; cluster->sockdir = user_opts.socketdir;
else else
{ {

View File

@ -65,7 +65,7 @@ static void create_new_objects(void);
static void copy_xact_xlog_xid(void); static void copy_xact_xlog_xid(void);
static void set_frozenxids(bool minmxid_only); static void set_frozenxids(bool minmxid_only);
static void make_outputdirs(char *pgdata); static void make_outputdirs(char *pgdata);
static void setup(char *argv0, bool *live_check); static void setup(char *argv0);
static void create_logical_replication_slots(void); static void create_logical_replication_slots(void);
ClusterInfo old_cluster, ClusterInfo old_cluster,
@ -88,7 +88,6 @@ int
main(int argc, char **argv) main(int argc, char **argv)
{ {
char *deletion_script_file_name = NULL; char *deletion_script_file_name = NULL;
bool live_check = false;
/* /*
* pg_upgrade doesn't currently use common/logging.c, but initialize it * pg_upgrade doesn't currently use common/logging.c, but initialize it
@ -123,18 +122,18 @@ main(int argc, char **argv)
*/ */
make_outputdirs(new_cluster.pgdata); make_outputdirs(new_cluster.pgdata);
setup(argv[0], &live_check); setup(argv[0]);
output_check_banner(live_check); output_check_banner();
check_cluster_versions(); check_cluster_versions();
get_sock_dir(&old_cluster, live_check); get_sock_dir(&old_cluster);
get_sock_dir(&new_cluster, false); get_sock_dir(&new_cluster);
check_cluster_compatibility(live_check); check_cluster_compatibility();
check_and_dump_old_cluster(live_check); check_and_dump_old_cluster();
/* -- NEW -- */ /* -- NEW -- */
@ -331,7 +330,7 @@ make_outputdirs(char *pgdata)
static void static void
setup(char *argv0, bool *live_check) setup(char *argv0)
{ {
/* /*
* make sure the user has a clean environment, otherwise, we may confuse * make sure the user has a clean environment, otherwise, we may confuse
@ -378,7 +377,7 @@ setup(char *argv0, bool *live_check)
pg_fatal("There seems to be a postmaster servicing the old cluster.\n" pg_fatal("There seems to be a postmaster servicing the old cluster.\n"
"Please shutdown that postmaster and try again."); "Please shutdown that postmaster and try again.");
else else
*live_check = true; user_opts.live_check = true;
} }
} }
@ -660,7 +659,7 @@ create_new_objects(void)
set_frozenxids(true); set_frozenxids(true);
/* update new_cluster info now that we have objects in the databases */ /* update new_cluster info now that we have objects in the databases */
get_db_rel_and_slot_infos(&new_cluster, false); get_db_rel_and_slot_infos(&new_cluster);
} }
/* /*

View File

@ -321,6 +321,7 @@ typedef struct
typedef struct typedef struct
{ {
bool check; /* check clusters only, don't change any data */ bool check; /* check clusters only, don't change any data */
bool live_check; /* check clusters only, old server is running */
bool do_sync; /* flush changes to disk */ bool do_sync; /* flush changes to disk */
transferMode transfer_mode; /* copy files or link them? */ transferMode transfer_mode; /* copy files or link them? */
int jobs; /* number of processes/threads to use */ int jobs; /* number of processes/threads to use */
@ -365,20 +366,20 @@ extern OSInfo os_info;
/* check.c */ /* check.c */
void output_check_banner(bool live_check); void output_check_banner(void);
void check_and_dump_old_cluster(bool live_check); void check_and_dump_old_cluster(void);
void check_new_cluster(void); void check_new_cluster(void);
void report_clusters_compatible(void); void report_clusters_compatible(void);
void issue_warnings_and_set_wal_level(void); void issue_warnings_and_set_wal_level(void);
void output_completion_banner(char *deletion_script_file_name); void output_completion_banner(char *deletion_script_file_name);
void check_cluster_versions(void); void check_cluster_versions(void);
void check_cluster_compatibility(bool live_check); void check_cluster_compatibility(void);
void create_script_for_old_cluster_deletion(char **deletion_script_file_name); void create_script_for_old_cluster_deletion(char **deletion_script_file_name);
/* controldata.c */ /* controldata.c */
void get_control_data(ClusterInfo *cluster, bool live_check); void get_control_data(ClusterInfo *cluster);
void check_control_data(ControlData *oldctrl, ControlData *newctrl); void check_control_data(ControlData *oldctrl, ControlData *newctrl);
void disable_old_cluster(void); void disable_old_cluster(void);
@ -427,7 +428,7 @@ void check_loadable_libraries(void);
FileNameMap *gen_db_file_maps(DbInfo *old_db, FileNameMap *gen_db_file_maps(DbInfo *old_db,
DbInfo *new_db, int *nmaps, const char *old_pgdata, DbInfo *new_db, int *nmaps, const char *old_pgdata,
const char *new_pgdata); const char *new_pgdata);
void get_db_rel_and_slot_infos(ClusterInfo *cluster, bool live_check); void get_db_rel_and_slot_infos(ClusterInfo *cluster);
int count_old_cluster_logical_slots(void); int count_old_cluster_logical_slots(void);
void get_subscription_count(ClusterInfo *cluster); void get_subscription_count(ClusterInfo *cluster);
@ -435,7 +436,7 @@ void get_subscription_count(ClusterInfo *cluster);
void parseCommandLine(int argc, char *argv[]); void parseCommandLine(int argc, char *argv[]);
void adjust_data_dir(ClusterInfo *cluster); void adjust_data_dir(ClusterInfo *cluster);
void get_sock_dir(ClusterInfo *cluster, bool live_check); void get_sock_dir(ClusterInfo *cluster);
/* relfilenumber.c */ /* relfilenumber.c */