From: | Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> |
---|---|
To: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> |
Cc: | Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>, alvherre(at)alvh(dot)no-ip(dot)org, pgsql-hackers(at)lists(dot)postgresql(dot)org |
Subject: | Re: A recent message added to pg_upgade |
Date: | 2023-10-30 07:08:47 |
Message-ID: | CALj2ACXQfR8OZZeJ708KFZs8DvMsN_7io_H5avwt6JDVSfNudQ@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Mon, Oct 30, 2023 at 8:51 AM Amit Kapila <amit(dot)kapila16(at)gmail(dot)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
From | Date | Subject | |
---|---|---|---|
Next Message | Michael Paquier | 2023-10-30 07:08:50 | Re: Requiring recovery.signal or standby.signal when recovering with a backup_label |
Previous Message | Bharath Rupireddy | 2023-10-30 06:29:10 | Re: Introduce a new view for checkpointer related stats |