On Tue, Sep 12, 2023 at 5:20 PM Hayato Kuroda (Fujitsu) <kuroda.hay...@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.