Date: Wed, 14 Sep 2022 14:17:46 -0700
From: Nathan Bossart <nathandbossart@gmail.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Peter Eisentraut <peter.eisentraut@enterprisedb.com>,
	Benoit =?iso-8859-1?Q?Lobr=E9au?= <benoit.lobreau@gmail.com>,
	Alvaro Herrera <alvherre@alvh.no-ip.org>,
	Robert Haas <robertmhaas@gmail.com>,
	"Bossart, Nathan" <bossartn@amazon.com>,
	David Steele <david@pgmasters.net>,
	Fujii Masao <masao.fujii@oss.nttdata.com>,
	"pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>,
	Michael Paquier <michael@paquier.xyz>,
	Magnus Hagander <magnus@hagander.net>,
	Stephen Frost <sfrost@snowman.net>
Subject: Re: archive modules
Message-ID: <20220914211746.GA2986134@nathanxps13>
References: <20220825200600.GA1465148@nathanxps13>
 <20220829224621.GB541895@nathanxps13>
 <CAPE8EZ7SQKKkreVT2Cjrheg8dChT_qSDRBS71r-r+girEMQ4bw@mail.gmail.com>
 <20220830163048.GA544233@nathanxps13>
 <613541.1662842656@sss.pgh.pa.us>
 <20220910211046.GA2651323@nathanxps13>
 <9ee5d180-2c32-a1ca-d3d7-63a723f68d9a@enterprisedb.com>
 <20220914200305.GA2984249@nathanxps13>
 <37857474-5caa-ccb3-92a3-150ddfe851ab@enterprisedb.com>
 <2884265.1663188443@sss.pgh.pa.us>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <2884265.1663188443@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/20220914211746.GA2986134%40nathanxps13>
Precedence: bulk

On Wed, Sep 14, 2022 at 04:47:23PM -0400, Tom Lane wrote:
> Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:
>> On 14.09.22 22:03, Nathan Bossart wrote:
>>> I originally did it this way, but changed it based on this feedback [0].  I
>>> have no problem with the general idea, but the recovery_target_* logic does
>>> have the following note:
>>> 
>>> * XXX this code is broken by design.  Throwing an error from a GUC assign
>>> * hook breaks fundamental assumptions of guc.c.  So long as all the variables
>>> * for which this can happen are PGC_POSTMASTER, the consequences are limited,
>>> * since we'd just abort postmaster startup anyway.  Nonetheless it's likely
>>> * that we have odd behaviors such as unexpected GUC ordering dependencies.
> 
>> Ah yes, that won't work.  But maybe we can just check it at run time, 
>> like in LoadArchiveLibrary().
> 
> Yeah, the objection there is only to trying to enforce such
> interrelationships in GUC hooks.  In this case it seems to me that
> we could easily check and complain at the point where we're about
> to use the GUC values.

I think the cleanest way to do something like that would be to load a
check_configured_cb that produces a WARNING.  IIRC failing in
LoadArchiveLibrary() would just cause the archiver process to restart over
and over.  HandlePgArchInterrupts() might need some work as well.

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com