> On 15 Sep 2023, at 11:38, bt23nguyent <bt23nguy...@oss.nttdata.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
