Re: basic_archive lost archive_directory

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