Re: A recent message added to pg_upgade
Amit Kapila <amit.kapila16@gmail.com>
From: Amit Kapila <amit.kapila16@gmail.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: Peter Smith <smithpb2250@gmail.com>,
Kyotaro Horiguchi <horikyota.ntt@gmail.com>, bharath.rupireddyforpostgres@gmail.com, alvherre@alvh.no-ip.org, pgsql-hackers@lists.postgresql.org
Date: 2023-11-03T02:11:20Z
Lists: pgsql-hackers
On Thu, Nov 2, 2023 at 2:36 PM Amit Kapila <amit.kapila16@gmail.com> wrote: > > On Thu, Nov 2, 2023 at 11:32 AM Michael Paquier <michael@paquier.xyz> wrote: > > > > On Thu, Nov 02, 2023 at 02:32:07PM +1100, Peter Smith wrote: > > > On Thu, Nov 2, 2023 at 2:25 PM Peter Smith <smithpb2250@gmail.com> wrote: > > >> Checking this patch yesterday prompted me to create a new thread > > >> questioning the inconsistencies of the "GUC names in messages". In > > >> that thread, Tom Lane replied and gave some background information [1] > > >> about the GUC name embedding versus substitution. In hindsight, I > > >> think your original message was fine as-is, but there seem to be > > >> examples of every kind of style, so whatever you do would have some > > >> precedent. > > >> > > >> The patch v4 LGTM. > > > > > > To clarify, all the current code LGTM, but the patch is still missing > > > a guc_hook test case, right? > > > > - NULL, NULL, NULL > > + check_max_slot_wal_keep_size, NULL, NULL > > > > FWIW, I am +-0 with what you are proposing here. I don't quite get > > why one may want to enforce this specific GUC at upgrade. > > > > I also can't think of a good reason to do so but OTOH, I can't imagine > all possible scenarios. As this setting is invalid or can cause > problems, it seems people favor preventing it. Alvaro also voted in > favor of preventing it, so we are considering to proceed with it > unless more people think otherwise. > Now, that Michael also committed another similar change in commit 7021d3b176, it is better to be consistent in both cases. So, either we should have check hooks for both parameters or follow another route where we always forcibly override these parameters (which means the user-provided values for these parameters will be ignored) in pg_upgrade and document it. Yet another simple way is to simply document the current behavior. In the future, if we see users complain about this or have use cases to use these parameters during an upgrade, we can accordingly try to adapt the behavior. Thoughts? -- With Regards, Amit Kapila.
Commits
-
Fix the handling of two GUCs during upgrade.
- 72e6c08fea7c 19 (unreleased) landed
- f36e5774510d 18.0 landed
- 24f6c1bd41d0 17.6 landed
-
Rework how logirep launchers are stopped during pg_upgrade
- 08c3ad27eb53 17.0 landed
-
Prohibit max_slot_wal_keep_size to value other than -1 during upgrade.
- 8bfb231b43d7 17.0 landed
-
Prevent startup of logical replication launcher during pg_upgrade
- 7021d3b17664 17.0 cited
-
Migrate logical slots to the new node during an upgrade.
- 29d0a77fa660 17.0 cited