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

diff --git a/src/bin/pg_upgrade/check.c b/src/bin/pg_upgrade/check.c
index 51e30a2f2399d16ed99e48dda99f8c56493ee698..5038231731ff262e8329f6a963d670c05ba798fd 100644 (file)
--- a/src/bin/pg_upgrade/check.c
+++ b/src/bin/pg_upgrade/check.c
@@ -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_new_cluster_logical_replication_slots(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);
 
 /*
@@ -555,9 +555,9 @@ fix_path_separator(char *path)
 }
 
 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,
               "Performing Consistency Checks on Old Live Server\n"
@@ -573,18 +573,18 @@ output_check_banner(bool live_check)
 
 
 void
-check_and_dump_old_cluster(bool live_check)
+check_and_dump_old_cluster(void)
 {
    /* -- OLD -- */
 
-   if (!live_check)
+   if (!user_opts.live_check)
        start_postmaster(&old_cluster, true);
 
    /*
     * Extract a list of databases, tables, and logical replication slots from
     * the old cluster.
     */
-   get_db_rel_and_slot_infos(&old_cluster, live_check);
+   get_db_rel_and_slot_infos(&old_cluster);
 
    init_tablespaces();
 
@@ -605,7 +605,7 @@ check_and_dump_old_cluster(bool live_check)
         * Logical replication slots can be migrated since PG17. See comments
         * 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.
@@ -669,7 +669,7 @@ check_and_dump_old_cluster(bool live_check)
    if (!user_opts.check)
        generate_old_dump();
 
-   if (!live_check)
+   if (!user_opts.live_check)
        stop_postmaster(false);
 }
 
@@ -677,7 +677,7 @@ check_and_dump_old_cluster(bool live_check)
 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();
 
@@ -828,14 +828,14 @@ check_cluster_versions(void)
 
 
 void
-check_cluster_compatibility(bool live_check)
+check_cluster_compatibility(void)
 {
    /* get/check pg_control data of servers */
-   get_control_data(&old_cluster, live_check);
-   get_control_data(&new_cluster, false);
+   get_control_data(&old_cluster);
+   get_control_data(&new_cluster);
    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, "
                 "the old and new port numbers must be different.");
 }
@@ -1838,7 +1838,7 @@ check_new_cluster_subscription_configuration(void)
  * before shutdown.
  */
 static void
-check_old_cluster_for_valid_slots(bool live_check)
+check_old_cluster_for_valid_slots(void)
 {
    char        output_path[MAXPGPATH];
    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
             * 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 &&
                    (script = fopen_priv(output_path, "w")) == NULL)

diff --git a/src/bin/pg_upgrade/controldata.c b/src/bin/pg_upgrade/controldata.c
index 1f0ccea3ed19b39633079277cb9d9ae26e836b51..854c6887a23e0175b928886bcbaa5e50bec0ca60 100644 (file)
--- a/src/bin/pg_upgrade/controldata.c
+++ b/src/bin/pg_upgrade/controldata.c
@@ -33,7 +33,7 @@
  * return valid xid data for a running server.
  */
 void
-get_control_data(ClusterInfo *cluster, bool live_check)
+get_control_data(ClusterInfo *cluster)
 {
    char        cmd[MAXPGPATH];
    char        bufin[MAX_STRING];
@@ -76,6 +76,7 @@ get_control_data(ClusterInfo *cluster, bool live_check)
    uint32      segno = 0;
    char       *resetwal_bin;
    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

diff --git a/src/bin/pg_upgrade/info.c b/src/bin/pg_upgrade/info.c
index c07a69b63e5103b5e52cfe57a474394585273a16..5de5e109452fe0bcc38316a2277abf9437fce545 100644 (file)
--- a/src/bin/pg_upgrade/info.c
+++ b/src/bin/pg_upgrade/info.c
@@ -27,7 +27,7 @@ static void free_rel_infos(RelInfoArr *rel_arr);
 static void print_db_infos(DbInfoArr *db_arr);
 static void print_rel_infos(RelInfoArr *rel_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
  * on the given "port". Assumes that server is already running.
- *
- * live_check would be used only when the target is the old cluster.
  */
 void
-get_db_rel_and_slot_infos(ClusterInfo *cluster, bool live_check)
+get_db_rel_and_slot_infos(ClusterInfo *cluster)
 {
    int         dbnum;
 
@@ -293,7 +291,7 @@ get_db_rel_and_slot_infos(ClusterInfo *cluster, bool live_check)
        get_rel_infos(cluster, pDbInfo);
 
        if (cluster == &old_cluster)
-           get_old_cluster_logical_slot_infos(pDbInfo, live_check);
+           get_old_cluster_logical_slot_infos(pDbInfo);
    }
 
    if (cluster == &old_cluster)
@@ -637,7 +635,7 @@ get_rel_infos(ClusterInfo *cluster, DbInfo *dbinfo)
  * are included.
  */
 static void
-get_old_cluster_logical_slot_infos(DbInfo *dbinfo, bool live_check)
+get_old_cluster_logical_slot_infos(DbInfo *dbinfo)
 {
    PGconn     *conn;
    PGresult   *res;
@@ -673,7 +671,7 @@ get_old_cluster_logical_slot_infos(DbInfo *dbinfo, bool live_check)
                            "WHERE slot_type = 'logical' AND "
                            "database = current_database() AND "
                            "temporary IS FALSE;",
-                           live_check ? "FALSE" :
+                           user_opts.live_check ? "FALSE" :
                            "(CASE WHEN invalidation_reason IS NOT NULL THEN FALSE "
                            "ELSE (SELECT pg_catalog.binary_upgrade_logical_slot_has_caught_up(slot_name)) "
                            "END)");

diff --git a/src/bin/pg_upgrade/option.c b/src/bin/pg_upgrade/option.c
index 548ea4e62365635e9f6600cf10b3d8f5b37fade1..6f41d63eed4319a90b10426f2b7bc6735ab41be6 100644 (file)
--- a/src/bin/pg_upgrade/option.c
+++ b/src/bin/pg_upgrade/option.c
@@ -470,10 +470,10 @@ adjust_data_dir(ClusterInfo *cluster)
  * directory.
  */
 void
-get_sock_dir(ClusterInfo *cluster, bool live_check)
+get_sock_dir(ClusterInfo *cluster)
 {
 #if !defined(WIN32)
-   if (!live_check)
+   if (!user_opts.live_check || cluster == &new_cluster)
        cluster->sockdir = user_opts.socketdir;
    else
    {

diff --git a/src/bin/pg_upgrade/pg_upgrade.c b/src/bin/pg_upgrade/pg_upgrade.c
index 03eb738fd7e62aefadc7b1ba630cbc9f94ec77c6..99f3d4543e94903d5cd94837a92c3992e2596dc3 100644 (file)
--- a/src/bin/pg_upgrade/pg_upgrade.c
+++ b/src/bin/pg_upgrade/pg_upgrade.c
@@ -65,7 +65,7 @@ static void create_new_objects(void);
 static void copy_xact_xlog_xid(void);
 static void set_frozenxids(bool minmxid_only);
 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);
 
 ClusterInfo old_cluster,
@@ -88,7 +88,6 @@ int
 main(int argc, char **argv)
 {
    char       *deletion_script_file_name = NULL;
-   bool        live_check = false;
 
    /*
     * 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);
 
-   setup(argv[0], &live_check);
+   setup(argv[0]);
 
-   output_check_banner(live_check);
+   output_check_banner();
 
    check_cluster_versions();
 
-   get_sock_dir(&old_cluster, live_check);
-   get_sock_dir(&new_cluster, false);
+   get_sock_dir(&old_cluster);
+   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 -- */
@@ -331,7 +330,7 @@ make_outputdirs(char *pgdata)
 
 
 static void
-setup(char *argv0, bool *live_check)
+setup(char *argv0)
 {
    /*
     * 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"
                         "Please shutdown that postmaster and try again.");
            else
-               *live_check = true;
+               user_opts.live_check = true;
        }
    }
 
@@ -660,7 +659,7 @@ create_new_objects(void)
        set_frozenxids(true);
 
    /* 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);
 }
 
 /*

diff --git a/src/bin/pg_upgrade/pg_upgrade.h b/src/bin/pg_upgrade/pg_upgrade.h
index f885cc9fc407fc06ef610f7e08a5e01ab0892807..cdb6e2b7597b12a58015ef17daa8900ec3188c8f 100644 (file)
--- a/src/bin/pg_upgrade/pg_upgrade.h
+++ b/src/bin/pg_upgrade/pg_upgrade.h
@@ -321,6 +321,7 @@ typedef struct
 typedef struct
 {
    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 */
    transferMode transfer_mode; /* copy files or link them? */
    int         jobs;           /* number of processes/threads to use */
@@ -365,20 +366,20 @@ extern OSInfo os_info;
 
 /* check.c */
 
-void       output_check_banner(bool live_check);
-void       check_and_dump_old_cluster(bool live_check);
+void       output_check_banner(void);
+void       check_and_dump_old_cluster(void);
 void       check_new_cluster(void);
 void       report_clusters_compatible(void);
 void       issue_warnings_and_set_wal_level(void);
 void       output_completion_banner(char *deletion_script_file_name);
 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);
 
 
 /* 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       disable_old_cluster(void);
 
@@ -427,7 +428,7 @@ void        check_loadable_libraries(void);
 FileNameMap *gen_db_file_maps(DbInfo *old_db,
                              DbInfo *new_db, int *nmaps, const char *old_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);
 void       get_subscription_count(ClusterInfo *cluster);
 
@@ -435,7 +436,7 @@ void        get_subscription_count(ClusterInfo *cluster);
 
 void       parseCommandLine(int argc, char *argv[]);
 void       adjust_data_dir(ClusterInfo *cluster);
-void       get_sock_dir(ClusterInfo *cluster, bool live_check);
+void       get_sock_dir(ClusterInfo *cluster);
 
 /* relfilenumber.c */