Horton

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

Amit Kapila <amit.kapila16@gmail.com>

View thread View on postgr.es
From: Amit Kapila <amit.kapila16@gmail.com>
To: "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>
Cc: vignesh C <vignesh21@gmail.com>, "Wei Wang (Fujitsu)" <wangw.fnst@fujitsu.com>, Peter Eisentraut <peter.eisentraut@enterprisedb.com>, Julien Rouhaud <rjuju123@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>, Peter Smith <smithpb2250@gmail.com>
Date: 2023-07-17T05:41:01Z
Lists: pgsql-hackers 
On Thu, Jun 8, 2023 at 9:24 AM Hayato Kuroda (Fujitsu)
<kuroda.hayato@fujitsu.com> wrote:
>

Few comments/questions
====================
1.
+check_for_parameter_settings(ClusterInfo *new_cluster)
{
...
+
+ res = executeQueryOrDie(conn, "SHOW max_replication_slots;");
+ max_replication_slots = atoi(PQgetvalue(res, 0, 0));
+
+ if (max_replication_slots == 0)
+ pg_fatal("max_replication_slots must be greater than 0");
...
}

Won't it be better to verify that the value of "max_replication_slots"
is greater than the number of logical slots we are planning to copy
from old on the new cluster? Similar to this, I thought whether we
need to check the value of max_wal_senders? But, I guess one can
simply decode from slots by using APIs, so not sure about that. What
do you think?

2.
+ /*
+ * Dump logical replication slots if needed.
+ *
+ * XXX We cannot dump replication slots at the same time as the schema
+ * dump because we need to separate the timing of restoring
+ * replication slots and other objects. Replication slots, in
+ * particular, should not be restored before executing the pg_resetwal
+ * command because it will remove WALs that are required by the slots.
+ */
+ if (user_opts.include_logical_slots)

Can you explain this point a bit more with some example scenarios?
Basically, if we had sent all the WAL before the upgrade then why do
we need to worry about the timing of pg_resetwal?

3. I see that you are trying to ensure that all the WAL has been
consumed for a slot except for shutdown_checkpoint in patch 0003 but
do we need to think of any interaction with restart_lsn
(MyReplicationSlot->data.restart_lsn) which is the start point to read
WAL for decoding by walsender?

-- 
With Regards,
Amit Kapila.

Commits

  1. Fix issues in binary_upgrade_logical_slot_has_caught_up().

  2. Fix a random failure in 003_logical_slots.pl.

  3. Fix a test in 003_logical_slots.

  4. Fix uninitialized slot array access during the upgrade.

  5. Fix the test 003_logical_slots.

  6. Commit b195e6d482 forgot to update meson.build.

  7. Use shorter file names in the upgrade logical slots test.

  8. Migrate logical slots to the new node during an upgrade.

  9. Flush logical slots to disk during a shutdown checkpoint if required.

  10. Prevent possibility of panics during shutdown checkpoint.