On Mon, Aug 21, 2023 at 6:32 PM Hayato Kuroda (Fujitsu)
<kuroda.hayato@fujitsu.com> wrote:
>
> > 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().
>
> Some checks were included to check_for_logical_replication_slots(), and
> get_logical_slot_infos() for new_cluster was removed as you said.
>
+ res = executeQueryOrDie(conn, "SELECT slot_name "
+ "FROM pg_catalog.pg_replication_slots "
+ "WHERE slot_type = 'logical' AND "
+ "temporary IS FALSE;");
+
+ if (PQntuples(res))
+ pg_fatal("New cluster must not have logical replication slot, but
found \"%s\"",
+ PQgetvalue(res, 0, 0));
+
+ PQclear(res);
+
+ nslots = count_logical_slots(&old_cluster);
+
+ /*
+ * Do additional checks when the logical replication slots have on the old
+ * cluster.
+ */
+ if (nslots)
Shouldn't these checks be reversed? I mean it would be better to test
the presence of slots on the new cluster if there is any slot present
on the old cluster.
--
With Regards,
Amit Kapila.