Date: Tue, 24 Feb 2026 11:34:16 -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: <aZ3hGHgdJDPuC6I1@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>
 <aYtXgaKfiYpaqQyW@nathan>
 <CAHGQGwFq-7p1nCfemJCe0_VXVWxpGdq2D4ss+ns6p7=W1FTDqg@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CAHGQGwFq-7p1nCfemJCe0_VXVWxpGdq2D4ss+ns6p7=W1FTDqg@mail.gmail.com>
Archived-At: <https://www.postgresql.org/message-id/aZ3hGHgdJDPuC6I1%40nathan>
Precedence: bulk

On Tue, Feb 24, 2026 at 01:28:50PM +0900, Fujii Masao wrote:
> I agree with removing the check from the GUC check hook. My only concern is
> that simply removing it (rather than relocating it) changes the error message
> users see when archive_directory is misconfigured, which may make
> troubleshooting WAL archiving failures slightly harder.
> 
> [Current]
> WARNING:  invalid value for parameter
> "basic_archive.archive_directory": "not_exists"
> DETAIL:  Specified archive directory does not exist.
> 
> [With the patch]
> ERROR:  could not create file
> "not_exists/archtemp.00000001000000000000000E.80107.1771905339058": No
> such file or directory
> 
> One option would be to perform the check in check_configured_cb(),
> but as you noted, that would add an extra stat() call for each WAL archiving.
> If that overhead is unacceptable, another approach would be to wrap
> copy_file() in basic_archive_file() with PG_TRY() / PG_CATCH(). On error,
> we could stat() the archive directory and report a clearer reason if it does
> not exist.
> 
> That said, if users are generally fine with the "could not create file" error,
> I'm ok with the proposed patch (i.e., just removing the check).

I think it's fine as-is.  If something is wrong with the error message,
IMHO we should fix the error message.  Adding extra stat() calls to try to
give a nicer message might work most of the time, but there are still race
conditions where users will see the original one.  But in any case, I
believe the current message style is used in many places, so I don't see a
strong reason to do anything different here.

-- 
nathan