Re: recovery modules

On Wed, Jan 25, 2023 at 04:34:21PM +0900, Michael Paquier wrote:
> The loop part is annoying..  I've never been a fan of adding this
> cross-value checks for the archiver modules in the first place, and it
> would make things much simpler in the checkpointer if we need to think
> about that as we want these values to be reloadable.  Perhaps this
> could just be an exception where we just give priority on one over the
> other archive_cleanup_command?  The startup process has a well-defined
> sequence after a failure, while the checkpointer is designed to remain
> robust.

Yeah, there are some problems here.  If we ERROR, we'll just bounce back to
the sigsetjmp() block once a second, and we'll never pick up configuration
reloads, shutdown signals, etc.  If we FATAL, we'll just rapidly restart
over and over.  Given the dicussion about misconfigured archiving
parameters [0], I doubt folks will be okay with giving priority to one or
the other.

I'm currently thinking that the checkpointer should set a flag and clear
the recovery callbacks when a misconfiguration is detected.  Anytime the
checkpointer tries to use the archive-cleanup callback, a WARNING would be
emitted.  This is similar to an approach I proposed for archiving
misconfigurations (that we didn't proceed with) [1].  Given the
aformentioned problems, this approach might be more suitable for the
checkpointer than it is for the archiver.

Thoughts?

[0] https://postgr.es/m/9ee5d180-2c32-a1ca-d3d7-63a723f68d9a%40enterprisedb.com
[1] https://postgr.es/m/20220914222736.GA3042279%40nathanxps13

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com