Re: basic_archive lost archive_directory

On Wed, Feb 25, 2026 at 2:34 AM Nathan Bossart <nathandbossart(at)gmail(dot)com> wrote:
>
> 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.

Ok, so since I seem to be the only one with some reservations about
the message, I'm fine with proceeding with the proposed change.

Regards,

--
Fujii Masao