On Tue, Jul 23, 2024 at 09:05:05AM +0530, Amit Kapila wrote:
> Right, the other option would be to move it to the place where we call
> check_old_cluster_for_valid_slots(), etc. Initially, it was kept in
> the specific function (get_db_rel_and_slot_infos) as we were
> mainlining the count at the per-database level but now as we are
> changing that I am not sure if calling it from the same place is a
> good idea. But OTOH, it is okay to keep it at the place where we
> retrieve the required information from the old cluster.
I moved it to where you suggested.
> One minor point is the comment atop get_subscription_count() still
> refers to the function name as get_db_subscription_count().
Oops, fixed.
--
nathan
>From 19831c5a2869f949e73564abea8a36858b39bcd1 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nat...@postgresql.org>
Date: Sat, 20 Jul 2024 21:01:29 -0500
Subject: [PATCH v3 1/1] pg_upgrade: retrieve subscription count more
efficiently
---
src/bin/pg_upgrade/check.c | 13 ++++-----
src/bin/pg_upgrade/info.c | 51 +++++----------------------------
src/bin/pg_upgrade/pg_upgrade.h | 4 +--
3 files changed, 15 insertions(+), 53 deletions(-)
diff --git a/src/bin/pg_upgrade/check.c b/src/bin/pg_upgrade/check.c
index 27924159d6..51e30a2f23 100644
--- a/src/bin/pg_upgrade/check.c
+++ b/src/bin/pg_upgrade/check.c
@@ -609,8 +609,10 @@ check_and_dump_old_cluster(bool live_check)
/*
* Subscriptions and their dependencies can be migrated since
PG17.
- * See comments atop get_db_subscription_count().
+ * Before that the logical slots are not upgraded, so we will
not be
+ * able to upgrade the logical replication clusters completely.
*/
+ get_subscription_count(&old_cluster);
check_old_cluster_subscription_state();
}
@@ -1797,17 +1799,14 @@ check_new_cluster_subscription_configuration(void)
{
PGresult *res;
PGconn *conn;
- int nsubs_on_old;
int max_replication_slots;
/* Subscriptions and their dependencies can be migrated since PG17. */
if (GET_MAJOR_VERSION(old_cluster.major_version) < 1700)
return;
- nsubs_on_old = count_old_cluster_subscriptions();
-
/* Quick return if there are no subscriptions to be migrated. */
- if (nsubs_on_old == 0)
+ if (old_cluster.nsubs == 0)
return;
prep_status("Checking for new cluster configuration for subscriptions");
@@ -1821,10 +1820,10 @@ check_new_cluster_subscription_configuration(void)
pg_fatal("could not determine parameter settings on new
cluster");
max_replication_slots = atoi(PQgetvalue(res, 0, 0));
- if (nsubs_on_old > max_replication_slots)
+ if (old_cluster.nsubs > max_replication_slots)
pg_fatal("\"max_replication_slots\" (%d) must be greater than
or equal to the number of "
"subscriptions (%d) on the old cluster",
- max_replication_slots, nsubs_on_old);
+ max_replication_slots, old_cluster.nsubs);
PQclear(res);
PQfinish(conn);
diff --git a/src/bin/pg_upgrade/info.c b/src/bin/pg_upgrade/info.c
index 95c22a7200..c07a69b63e 100644
--- a/src/bin/pg_upgrade/info.c
+++ b/src/bin/pg_upgrade/info.c
@@ -28,7 +28,6 @@ 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_db_subscription_count(DbInfo *dbinfo);
/*
@@ -293,15 +292,8 @@ get_db_rel_and_slot_infos(ClusterInfo *cluster, bool
live_check)
get_rel_infos(cluster, pDbInfo);
- /*
- * Retrieve the logical replication slots infos and the
subscriptions
- * count for the old cluster.
- */
if (cluster == &old_cluster)
- {
get_old_cluster_logical_slot_infos(pDbInfo, live_check);
- get_db_subscription_count(pDbInfo);
- }
}
if (cluster == &old_cluster)
@@ -748,54 +740,25 @@ count_old_cluster_logical_slots(void)
}
/*
- * get_db_subscription_count()
- *
- * Gets the number of subscriptions in the database referred to by "dbinfo".
+ * get_subscription_count()
*
- * Note: This function will not do anything if the old cluster is pre-PG17.
- * This is because before that the logical slots are not upgraded, so we will
- * not be able to upgrade the logical replication clusters completely.
+ * Gets the number of subscriptions in the cluster.
*/
-static void
-get_db_subscription_count(DbInfo *dbinfo)
+void
+get_subscription_count(ClusterInfo *cluster)
{
PGconn *conn;
PGresult *res;
- /* Subscriptions can be migrated since PG17. */
- if (GET_MAJOR_VERSION(old_cluster.major_version) < 1700)
- return;
-
- conn = connectToServer(&old_cluster, dbinfo->db_name);
+ conn = connectToServer(cluster, "template1");
res = executeQueryOrDie(conn, "SELECT count(*) "
- "FROM
pg_catalog.pg_subscription WHERE subdbid = %u",
- dbinfo->db_oid);
- dbinfo->nsubs = atoi(PQgetvalue(res, 0, 0));
+ "FROM
pg_catalog.pg_subscription");
+ cluster->nsubs = atoi(PQgetvalue(res, 0, 0));
PQclear(res);
PQfinish(conn);
}
-/*
- * count_old_cluster_subscriptions()
- *
- * Returns the number of subscriptions for all databases.
- *
- * Note: this function always returns 0 if the old_cluster is PG16 and prior
- * because we gather subscriptions only for cluster versions greater than or
- * equal to PG17. See get_db_subscription_count().
- */
-int
-count_old_cluster_subscriptions(void)
-{
- int nsubs = 0;
-
- for (int dbnum = 0; dbnum < old_cluster.dbarr.ndbs; dbnum++)
- nsubs += old_cluster.dbarr.dbs[dbnum].nsubs;
-
- return nsubs;
-}
-
static void
free_db_and_rel_infos(DbInfoArr *db_arr)
{
diff --git a/src/bin/pg_upgrade/pg_upgrade.h b/src/bin/pg_upgrade/pg_upgrade.h
index 8afe240bdf..e2b99b49fa 100644
--- a/src/bin/pg_upgrade/pg_upgrade.h
+++ b/src/bin/pg_upgrade/pg_upgrade.h
@@ -197,7 +197,6 @@ typedef struct
* path */
RelInfoArr rel_arr; /* array of all user relinfos */
LogicalSlotInfoArr slot_arr; /* array of all LogicalSlotInfo */
- int nsubs; /* number of
subscriptions */
} DbInfo;
/*
@@ -296,6 +295,7 @@ typedef struct
char major_version_str[64]; /* string PG_VERSION of cluster
*/
uint32 bin_version; /* version returned from pg_ctl */
const char *tablespace_suffix; /* directory specification */
+ int nsubs; /* number of
subscriptions */
} ClusterInfo;
@@ -430,7 +430,7 @@ FileNameMap *gen_db_file_maps(DbInfo *old_db,
const char
*new_pgdata);
void get_db_rel_and_slot_infos(ClusterInfo *cluster, bool
live_check);
int count_old_cluster_logical_slots(void);
-int count_old_cluster_subscriptions(void);
+void get_subscription_count(ClusterInfo *cluster);
/* option.c */
--
2.39.3 (Apple Git-146)
