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

On Tue, Sep 12, 2023 at 5:20 PM Hayato Kuroda (Fujitsu)
<kuroda.hayato@fujitsu.com> wrote:
>

Few comments:
=============
1. One thing to note is that if user checks whether the old cluster is
upgradable with --check option and then try to upgrade, that will also
fail. Because during the --check run there would at least one
additional shutdown checkpoint WAL and then in the next run the slots
position won't match. Note, I am saying this in context of using
--check option with not-running old cluster. Won't that be surprising
to users? One possibility is that we document such a behaviour and
other is that we go back to WAL reading design where we can ignore
known WAL records like shutdown checkpoint, XLOG_RUNNING_XACTS, etc.

2.
+ /*
+ * Store the names of output plugins as well. There is a possibility
+ * that duplicated plugins are set, but the consumer function
+ * check_loadable_libraries() will avoid checking the same library, so
+ * we do not have to consider their uniqueness here.
+ */
+ for (slotno = 0; slotno < slot_arr->nslots; slotno++)
+ {
+ os_info.libraries[totaltups].name = pg_strdup(slot_arr->slots[slotno].plugin);

Here, we should ignore invalid slots.

3.
+ if (!live_check && !slot->caught_up)
+ {
+ if (script == NULL &&
+ (script = fopen_priv(output_path, "w")) == NULL)
+ pg_fatal("could not open file \"%s\": %s",
+ output_path, strerror(errno));
+
+ fprintf(script,
+ "The slot \"%s\" has not consumed the WAL yet\n",
+ slot->slotname);

Is it possible to print the LSN locations of slot and last checkpoint?
I think that will aid in debugging the problems if any and could be
helpful to users as well.

--
With Regards,
Amit Kapila.