From: Nathan Bossart <nathandbossart@gmail.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Jelte Fennema-Nio <postgres@jeltef.nl>
Cc: Robert Haas <robertmhaas@gmail.com>
Cc: Fujii Masao <masao.fujii@oss.nttdata.com>
Cc: pgsql-hackers@lists.postgresql.org
Subject: Re: Add a GUC check hook to ensure summarize_wal cannot be enabled when wal_level is minimal
Date: Wed, 10 Jul 2024 11:15:59 -0500
Message-ID: <Zo6zv2XGGJJugTOF@nathan> (raw)
In-Reply-To: <1749638.1720626878@sss.pgh.pa.us>
References: <802ff3d5-fdc7-4b08-9046-2e9fc5fdae0e@oss.nttdata.com>
<CA+TgmoYPNfgAT3OubjoYSiXTb3-urOjQxT3SCEUCrRBx5g6ihA@mail.gmail.com>
<CA+TgmoY8DnzWNK_xu4=NTYdxGBGnFucrEpOALnS0awMp-s_pKQ@mail.gmail.com>
<7c1b48d8-0f08-4733-a4e6-d55f04581db7@oss.nttdata.com>
<CA+TgmoansG0B8ayQGw9Ozee6Jawz=HFUACpO_4gp-+SFJBxiYw@mail.gmail.com>
<Zo6YPuvTmRW3b1s7@nathan>
<CAGECzQQcLYiAdRj5xAm1G43MtpD_rm029ziqrbpqyDdxaCoEQA@mail.gmail.com>
<1744272.1720624273@sss.pgh.pa.us>
<Zo6sTje4m0czRor9@nathan>
<1749638.1720626878@sss.pgh.pa.us>
On Wed, Jul 10, 2024 at 11:54:38AM -0400, Tom Lane wrote:
> Nathan Bossart <nathandbossart@gmail.com> writes:
>> I haven't tested it, but from skimming around the code, it looks like
>> ProcessConfigFileInternal() would deduplicate any previous entries in the
>> file prior to applying the values and running the check hooks. Else,
>> reloading a configuration file with multiple startup-only GUC entries could
>> fail, even without bogus GUC check hooks.
>
> While it's been a little while since I actually traced the logic,
> I believe the reason that case doesn't fail is this bit in
> set_config_with_handle, about line 3477 as of HEAD:
>
> case PGC_POSTMASTER:
> if (context == PGC_SIGHUP)
> {
> /*
> * We are re-reading a PGC_POSTMASTER variable from
> * postgresql.conf. We can't change the setting, so we should
> * give a warning if the DBA tries to change it. However,
> * because of variant formats, canonicalization by check
> * hooks, etc, we can't just compare the given string directly
> * to what's stored. Set a flag to check below after we have
> * the final storable value.
> */
> prohibitValueChange = true;
> }
> else if (context != PGC_POSTMASTER)
> // throw "cannot be changed now" error
That's what I thought at first, but then I saw this in
ProcessConfigFileInternal():
/* If it's already marked, then this is a duplicate entry */
if (record->status & GUC_IS_IN_FILE)
{
/*
* Mark the earlier occurrence(s) as dead/ignorable. We could
* avoid the O(N^2) behavior here with some additional state,
* but it seems unlikely to be worth the trouble.
*/
ConfigVariable *pitem;
for (pitem = head; pitem != item; pitem = pitem->next)
{
if (!pitem->ignore &&
strcmp(pitem->name, item->name) == 0)
pitem->ignore = true;
}
}
--
nathan
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: nathandbossart@gmail.com, tgl@sss.pgh.pa.us, postgres@jeltef.nl, robertmhaas@gmail.com, masao.fujii@oss.nttdata.com, pgsql-hackers@lists.postgresql.org
Subject: Re: Add a GUC check hook to ensure summarize_wal cannot be enabled when wal_level is minimal
In-Reply-To: <Zo6zv2XGGJJugTOF@nathan>
* 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