Re: [PoC] pg_upgrade: allow to upgrade publisher node

On Fri, Aug 18, 2023 at 7:21 PM Hayato Kuroda (Fujitsu)
<kuroda.hayato@fujitsu.com> wrote:
>

Few comments on new patches:
1.
+     <link linkend="sql-altersubscription"><command>ALTER
SUBSCRIPTION ... DISABLE</command></link>.
+     After the upgrade is complete, execute the
+     <command>ALTER SUBSCRIPTION ... CONNECTION</command> command to update the
+     connection string, and then re-enable the subscription.

Why does one need to update the connection string?

2.
+ /*
+ * Checking for logical slots must be done before
+ * check_new_cluster_is_empty() because the slot_arr attribute of the
+ * new_cluster will be checked in that function.
+ */
+ if (count_logical_slots(&old_cluster))
+ {
+ get_logical_slot_infos(&new_cluster, false);
+ check_for_logical_replication_slots(&new_cluster);
+ }
+
  check_new_cluster_is_empty();

Can't we simplify this checking by simply querying
pg_replication_slots for any usable slot something similar to what we
are doing in check_for_prepared_transactions()? We can add this check
in the function check_for_logical_replication_slots(). Also, do we
need a count function, or instead can we have a simple function like
is_logical_slot_present() where we return even if there is one slot
present?

Apart from this, (a) I have made a few changes (changed comments) in
patch 0001 as shared in the email [1]; (b) some modifications in the
docs as you can see in the attached. Please include those changes in
the next version if you think they are okay.

[1] - https://www.postgresql.org/message-id/CAA4eK1JzJagMmb_E8D4au%3DGYQkxox0AfNBm1FbP7sy7t4YWXPQ%40mail.gmail.com

--
With Regards,
Amit Kapila.