From: Tom Lane <tgl@sss.pgh.pa.us>
To: Alvaro Herrera <alvherre@alvh.no-ip.org>
Cc: Pg Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: pg_settings.pending_restart not set when line removed
Date: Tue, 27 Jul 2021 11:00:36 -0400
Message-ID: <273674.1627398036@sss.pgh.pa.us> (raw)
In-Reply-To: <202107271429.4m5ngdum7f66@alvherre.pgsql>
References: <202107271429.4m5ngdum7f66@alvherre.pgsql>
Alvaro Herrera <alvherre@alvh.no-ip.org> writes:
> On 2021-Jul-27, Tom Lane wrote:
>> Ugh. I think this patch is likely to create more problems than it fixes.
> I doubt that; as I said, the code already behaves in exactly that way
> for closely related operations, so this patch isn't doing anything new.
> Note that that loop this code is modifying only applies to lines that
> are removed from the config file.
Ah ... what's wrong here is some combination of -ENOCAFFEINE and a
not-great explanation on your part. I misread the patch as adding
"error = true" rather than the flag change. I agree that setting
the GUC_PENDING_RESTART flag is fine, because set_config_option
would do so if we reached it. Perhaps you should comment this
along that line? Also, the cases inside set_config_option
uniformly set that flag *before* the ereport not after.
So maybe like
if (gconf->context < PGC_SIGHUP)
{
+ /* The removal can't be effective without a restart */
+ gconf->status |= GUC_PENDING_RESTART;
ereport(elevel,
(errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
One thing worth checking is whether the pending-restart flag
gets cleared again if the DBA undoes the removal and again
reloads. I think the right thing will happen, but it'd be
worthwhile to check.
regards, tom lane
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: tgl@sss.pgh.pa.us, alvherre@alvh.no-ip.org, pgsql-hackers@lists.postgresql.org
Subject: Re: pg_settings.pending_restart not set when line removed
In-Reply-To: <273674.1627398036@sss.pgh.pa.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox