Re: A recent message added to pg_upgade - Mailing list pgsql-hackers

From Bharath Rupireddy
Subject Re: A recent message added to pg_upgade
Date
Msg-id CALj2ACXQfR8OZZeJ708KFZs8DvMsN_7io_H5avwt6JDVSfNudQ@mail.gmail.com
Whole thread Raw
In response to Re: A recent message added to pg_upgade  (Amit Kapila <amit.kapila16@gmail.com>)
Responses Re: A recent message added to pg_upgade
RE: A recent message added to pg_upgade
List pgsql-hackers
On Mon, Oct 30, 2023 at 8:51 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> > This discussion seems like a bit off from my point. I suggested adding
> > a check for that setting when IsBinaryUpgraded is true at the GUC
> > level as shown in the attached patch. I believe Álvaro made a similar
> > suggestion.  While the error message is somewhat succinct, I think it
> > is sufficient given the low possilibility of the scenario and the fact
> > that it cannot occur inadvertently.
> >
>
> I think we can simply change that error message to assert if we want
> to go with the check hook idea of yours. BTW, can we add
> GUC_check_errdetail() with a better message as some of the other check
> function uses? Also, I guess we can add some comments or at least
> refer to the existing comments to explain the reason of such a check.

Will the check_hook approach work correctly? I haven't checked that by
myself, but I see InitializeGUCOptions() getting called before
IsBinaryUpgrade is set to true and the passed-in config options ('c')
are parsed.

If the check_hook approach works correctly, I think we must add a test
hitting the error in check_max_slot_wal_keep_size for the
IsBinaryUpgrade case. And, I agree with Amit to have a detailed
messaging with GUC_check_errmsg/GUC_check_errdetail. Also, IMV,
leaving the error message in InvalidatePossiblyObsoleteSlot() there
(if required with a better wording as discussed initially in this
thread) does no harm. Actually, it acts as another safety net given
that max_slot_wal_keep_size GUC is reloadable via SIGHUP.

--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com



pgsql-hackers by date:

Previous
From: Bharath Rupireddy
Date:
Subject: Re: Introduce a new view for checkpointer related stats
Next
From: Michael Paquier
Date:
Subject: Re: Requiring recovery.signal or standby.signal when recovering with a backup_label