On Fri, Aug 18, 2023 at 7:21 PM Hayato Kuroda (Fujitsu) <kuroda.hay...@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.
mod_amit_1.patch
Description: Binary data