Re: Improve the log message output of basic_archive when basic_archive.archive_directory parameter is not set

> On 15 Sep 2023, at 11:38, bt23nguyent <bt23nguyent(at)oss(dot)nttdata(dot)com> wrote:
>
> Hi,
>
> When archive_library is set to 'basic_archive' but basic_archive.archive_directory is not set, WAL archiving doesn't work and only the following warning message is logged.
>
> $ emacs $PGDATA/postgresql.conf
> archive_mode = on
> archive_library = 'basic_archive'
>
> $ bin/pg_ctl -D $PGDATA restart
> ....
> WARNING: archive_mode enabled, yet archiving is not configured
>
> The issue here is that this warning message doesn't suggest any hint regarding the cause of WAL archiving failure. In other words, I think that the log message in this case should report that WAL archiving failed because basic_archive.archive_directory is not set.

That doesn't seem unreasonable, and I can imagine other callbacks having the
need to give errhints as well to assist the user.

> Thus, I think it's worth implementing new patch that improves that warning message, and here is the patch for that.

-basic_archive_configured(ArchiveModuleState *state)
+basic_archive_configured(ArchiveModuleState *state, const char **errmsg)

The variable name errmsg implies that it will contain the errmsg() data when it
in fact is used for errhint() data, so it should be named accordingly.

It's probably better to define the interface as ArchiveCheckConfiguredCB
functions returning an allocated string in the passed pointer which the caller
is responsible for freeing.

--
Daniel Gustafsson