RE: A recent message added to pg_upgade

Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com>

From: "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>
To: 'Kyotaro Horiguchi' <horikyota.ntt@gmail.com>, "bharath.rupireddyforpostgres@gmail.com" <bharath.rupireddyforpostgres@gmail.com>
Cc: "amit.kapila16@gmail.com" <amit.kapila16@gmail.com>, "alvherre@alvh.no-ip.org" <alvherre@alvh.no-ip.org>, "pgsql-hackers@lists.postgresql.org" <pgsql-hackers@lists.postgresql.org>
Date: 2023-10-31T13:44:07Z
Lists: pgsql-hackers

Attachments

Dear Horiguchi-san,

Thanks for making the patch!

> > 4. I think a test case to hit the error in the check hook in
> > 003_logical_slots.pl will help a lot here - not only covers the code
> > but also helps demonstrate how one can reach the error.
> 
> Yeah, of course. I was planning to add tests once the direction of the
> discussion became clear. I will add them in the next version.

I tried to make the part. Feel free to include it if not yet. We can check the
server log, but I think it may be overkill.

Also, I have one comment.

```
+bool
+check_max_slot_wal_keep_size(int *newval, void **extra, GucSource source)
+{
+    if (IsBinaryUpgrade && *newval != -1)
+    {
+        GUC_check_errdetail("\"max_slot_wal_keep_size\" must be set to -1 during binary upgrade mode.");
+        return false;
+    }
+    return true;
+}
```

Just to confirm - should we check the GucSource? Based on ur requirement, it might
be enough we avoid overwriting while starting the server.
Personally current code is OK because it is simpler, but I want to hear your opinion.

Best Regards,
Hayato Kuroda
FUJITSU LIMITED

Commits

  1. Fix the handling of two GUCs during upgrade.

  2. Rework how logirep launchers are stopped during pg_upgrade

  3. Prohibit max_slot_wal_keep_size to value other than -1 during upgrade.

  4. Prevent startup of logical replication launcher during pg_upgrade

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