public inbox for [email protected]  
help / color / mirror / Atom feed
From: Nathan Bossart <[email protected]>
To: Robert Haas <[email protected]>
Cc: Fujii Masao <[email protected]>
Cc: [email protected]
Subject: Re: Add a GUC check hook to ensure summarize_wal cannot be enabled when wal_level is minimal
Date: Mon, 15 Jul 2024 14:14:06 -0500
Message-ID: <ZpV0_gkCyGYooEZD@nathan> (raw)
In-Reply-To: <ZpVushkGx0_nYIip@nathan>
References: <CA+TgmoYPNfgAT3OubjoYSiXTb3-urOjQxT3SCEUCrRBx5g6ihA@mail.gmail.com>
	<CA+TgmoY8DnzWNK_xu4=NTYdxGBGnFucrEpOALnS0awMp-s_pKQ@mail.gmail.com>
	<[email protected]>
	<CA+TgmoansG0B8ayQGw9Ozee6Jawz=HFUACpO_4gp-+SFJBxiYw@mail.gmail.com>
	<CA+TgmoZ53ADUm2UAGEspMuv-Db5SGtrOXUx=ps6LcGexsf5-fA@mail.gmail.com>
	<[email protected]>
	<CA+TgmoYFOWbxk=FP+ZnxwVaKQEce=+0Xvzh0DUC9=YLNHiQW1w@mail.gmail.com>
	<[email protected]>
	<CA+TgmoYUhRa8j8_Jdrb9Ankz=c8NRLe_x1nf4L+Vc2V+DiTSWw@mail.gmail.com>
	<ZpVushkGx0_nYIip@nathan>

On Mon, Jul 15, 2024 at 01:47:14PM -0500, Nathan Bossart wrote:
> On Mon, Jul 15, 2024 at 02:30:42PM -0400, Robert Haas wrote:
>> I guess I'm in the group of people who doesn't understand how this can
>> possibly work. There's no guarantee about the order in which GUC check
>> hooks are called, so you don't know if the value of the other variable
>> has already been set to the final value or not, which seems like a
>> fatal problem even if the code happens to work correctly as of today.
>> Even if you have such a guarantee, you can't prohibit a configuration
>> change at pg_ctl reload time: the server can refuse to start in case
>> of an invalid configuration, but a running server can't decide to shut
>> down or stop working at reload time.
> 
> My understanding is that the correctness of this GUC check hook depends on
> wal_level being a PGC_POSTMASTER GUC.  The check hook would always return
> true during startup, and there'd be an additional cross-check in
> PostmasterMain() that would fail startup if necessary.  After that point,
> we know that wal_level cannot change, so the GUC check hook for
> summarize_wal can depend on wal_level.  If it fails, my expectation would
> be that the server would just ignore that change and continue.

I should also note that since wal_level defaults to "replica", I don't
think we even need any extra "always return true on startup" logic.  If
wal_level is set prior to summarize_wal, the check hook will fail startup
as needed.  If summarize_wal is set first, the check hook will return true,
and we'll fall back on the PostmasterMain() check (that already exists).

In short, the original patch [0] seems like it should work in this
particular scenario, barring some corner case I haven't discovered.  That
being said, it's admittedly fragile and probably not a great precedent to
set.  I've been thinking about some ideas for more generic GUC dependency
tooling, but I don't have anything to share yet.

[0] https://postgr.es/m/attachment/161852/v1-0001-Prevent-summarize_wal-from-enabling-when-wal_leve.patc...

-- 
nathan






view thread (2+ messages)  latest in thread

reply

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: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Add a GUC check hook to ensure summarize_wal cannot be enabled when wal_level is minimal
  In-Reply-To: <ZpV0_gkCyGYooEZD@nathan>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox