Date: Tue, 12 Apr 2022 13:08:35 +0800
From: Julien Rouhaud <rjuju123@gmail.com>
To: Nathan Bossart <nathandbossart@gmail.com>
Cc: Robert Haas <robertmhaas@gmail.com>, Andres Freund <andres@anarazel.de>,
	Michael Paquier <michael@paquier.xyz>,
	"Bossart, Nathan" <bossartn@amazon.com>,
	Fujii Masao <masao.fujii@oss.nttdata.com>,
	"wangsh.fnst@fujitsu.com" <wangsh.fnst@fujitsu.com>,
	Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>,
	Greg Sabino Mullane <htamfids@gmail.com>,
	Tom Lane <tgl@sss.pgh.pa.us>,
	"pgsql-hackers@lists.postgresql.org" <pgsql-hackers@lists.postgresql.org>
Subject: Re: make MaxBackends available in _PG_init
Message-ID: <YlUJU27iM3CEg6w3@jrouhaud>
References: <20220326072203.asge6bua7dxczf7m@jrouhaud>
 <20220326172316.obspcdmjqerislri@alap3.anarazel.de>
 <CA+Tgmoawc-4i3uFkNdwp8wUR9-jmfV0xjKDyJvGR9t2k2f=1EQ@mail.gmail.com>
 <20220330163051.GA784643@nathanxps13>
 <YlGJGiofZiWN3elx@jrouhaud>
 <CA+TgmoZRnwF=9tMX0TiqZGC4qAK_27NweKd5r-S=OSaE_nL70A@mail.gmail.com>
 <20220411164408.GA1915258@nathanxps13>
 <CA+TgmoZVQznhWpUr-bjC9U=Won8ZnfozSkiXrW2HU80r6QyASQ@mail.gmail.com>
 <20220411204442.GA2004824@nathanxps13>
 <20220411211435.GA2057981@nathanxps13>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220411211435.GA2057981@nathanxps13>
Archived-At: 
 <https://www.postgresql.org/message-id/YlUJU27iM3CEg6w3%40jrouhaud>
Precedence: bulk

Hi,

On Mon, Apr 11, 2022 at 02:14:35PM -0700, Nathan Bossart wrote:
> On Mon, Apr 11, 2022 at 01:44:42PM -0700, Nathan Bossart wrote:
> > On Mon, Apr 11, 2022 at 04:36:36PM -0400, Robert Haas wrote:
> >> If we throw an error while defining_custom_guc is true, how will it
> >> ever again become false?
> > 
> > Ah, I knew I was forgetting something this morning.
> > 
> > It won't, but the only place it is presently needed is when loading
> > shared_preload_libraries, so I believe startup will fail anyway.  However,
> > I can see defining_custom_guc being used elsewhere, so that is probably not
> > good enough.  Another approach could be to add a static
> > set_config_option_internal() function with a boolean argument to indicate
> > whether it is being used while defining a custom GUC.  I'll adjust 0003
> > with that approach unless a better idea prevails.
> 
> Here's a new patch set.  I've only changed 0003 as described above.  My
> apologies for the unnecessary round trip.

It looks sensible to me, although I think I would apply 0003 before 0002.

+   if (process_shared_preload_libraries_in_progress &&
+       !allow_when_loading_preload_libs)
+       ereport(ERROR,
+               (errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
+                errmsg("cannot change parameters while loading "
+                       "\"shared_preload_libraries\"")));
+

I think the error message should mention at least which GUC is being modified.