Re: archive modules

On Sat, Oct 15, 2022 at 3:13 AM Nathan Bossart <nathandbossart(at)gmail(dot)com> wrote:
>
> On Fri, Oct 14, 2022 at 11:51:30AM -0700, Nathan Bossart wrote:
> > On Fri, Oct 14, 2022 at 12:10:18PM +0530, Bharath Rupireddy wrote:
> >> 2) I think we have a problem - set archive_mode and archive_library
> >> and start the server, then set archive_command, reload the conf, see
> >> [3] - the archiver needs to error out right? The archiver gets
> >> restarted whenever archive_library changes but not when
> >> archive_command changes. I think the right place for the error is
> >> after or at the end of HandlePgArchInterrupts().
> >
> > Good catch. You are right, this is broken. I believe that we need to
> > check for the misconfiguration in HandlePgArchInterrupts() in addition to
> > LoadArchiveLibrary(). I will work on fixing this.
>
> As promised...

Thanks. I think that if the condition can be simplified something like
in the attached. It's okay to call shutdown callback twice by getting
rid of the comment [1] as it doesn't add any extra value or
information, it just says that we're calling shutdown callback
function. With the attached, the code is more readable and the
footprint of the changes are reduced.

[1]
/*
* Call the currently loaded archive module's shutdown callback,
* if one is defined.
*/
call_archive_module_shutdown_callback(0, 0);

--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com