Date: Tue, 10 Feb 2026 10:06:25 -0600
From: Nathan Bossart <nathandbossart@gmail.com>
To: Fujii Masao <masao.fujii@gmail.com>
Cc: Sergei Kornilov <sk@zsrv.org>,
	=?utf-8?B?0J7Qu9C10LMg0KHQsNC80L7QudC70L7Qsg==?= <splarv@ya.ru>,
	pgsql-bugs@lists.postgresql.org,
	=?utf-8?Q?=C3=81lvaro?= Herrera <alvherre@kurilemu.de>
Subject: Re: basic_archive lost archive_directory
Message-ID: <aYtXgaKfiYpaqQyW@nathan>
References: <202601301728.sfkizrto3t5i@alvherre.pgsql>
 <9b9341b0-942e-4d34-b94f-92bd918fad04@ya.ru>
 <1317421770387925@cea5cfd9-50d3-4d85-a924-a7cc75f8f215>
 <CAHGQGwF3tFP=oQgYHoS3h9TMeFbZwXc6+xdPTW8CkMV2TkjTmg@mail.gmail.com>
 <aYpYaRipZ_-lYxNY@nathan>
 <CAHGQGwFzC+HYEwJGhpmu1NBiQp_Ln8t0z9CDt8QCQDnUtFAFYA@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CAHGQGwFzC+HYEwJGhpmu1NBiQp_Ln8t0z9CDt8QCQDnUtFAFYA@mail.gmail.com>
Archived-At: <https://www.postgresql.org/message-id/aYtXgaKfiYpaqQyW%40nathan>
Precedence: bulk

On Tue, Feb 10, 2026 at 10:23:02AM +0900, Fujii Masao wrote:
> You're right if an invalid value for basic_archive.archive_directory is detected
> at server startup. However, when the setting is changed and the configuration
> is reloaded, the default behavior does not emit an error log.

Oh, I see.  Even so, I don't think this problem is best fixed by moving
code from the GUC check hook to the archive module check callback.  For
starters, since the archive module check callback is called for each file
to archive, this adds unnecessary overhead to repeatedly verify things that
really shouldn't change as long as the GUC's value stays the same.  In
practice, the check callback is mostly meant to ensure the user hasn't
temporarily halted archiving, as per the following note in basic_archive's
documentation:

    The default is an empty string, which effectively halts WAL archiving,
    but if archive_mode is enabled, the server will accumulate WAL segment
    files in the expectation that a value will soon be provided.

Furthermore, fixing this problem in basic_archive doesn't fix it for other
archive modules that use GUC check hooks.  I think it should be fixed more
generally, perhaps by bumping up the log level in the archiver when the
GUC's prefix matches the archive_library.

I also want to push back a bit on the idea that basic_archive not working
when the directory is missing at startup is a bug.  The documentation for
basic_archive clearly states that the directory must already exist.  That
being said, I have no objection to making basic_archive more lenient or
even to back-patching such a change.  As I mentioned upthread, IMHO we
should simply remove the existence check from the GUC check hook.
basic_archive must already be written to handle the archive directory
disappearing at any moment, so we should be able to rely on it without the
extra stat().

-- 
nathan