On Tue, Dec 27, 2022 at 02:11:11PM -0800, Andres Freund wrote:
> On 2022-12-27 11:24:49 -0800, Nathan Bossart wrote:
>> * Unlike archive modules, recovery libraries cannot be changed at runtime.
>> There isn't a safe way to unload a library, and archive libraries work
>> around this restriction by restarting the archiver process. Since recovery
>> libraries are loaded via the startup and checkpointer processes (which
>> cannot be trivially restarted like the archiver), the same workaround is
>> not feasible.
>
> I don't think that's a convincing reason to not support configuration
> changes. Sure, libraries cannot be unloaded, but an unnecessarily loaded
> library is cheap. All that's needed is to redirect the relevant function
> calls.
Agreed. That seems worth the cost to switching this stuff to be
SIGHUP-able.
>> * pg_rewind uses restore_command, but there isn't a straightforward path to
>> support restore_library. I haven't addressed this in the attached patches,
>> but perhaps this is a reason to allow specifying both restore_command and
>> restore_library at the same time. pg_rewind would use restore_command, and
>> the server would use restore_library.
>
> That seems problematic, leading to situations where one might not be able to
> use restore_command anymore, because it's not feasible to do
> segment-by-segment restoration.
Do you mean that supporting restore_library in pg_rewind is a hard
requirement? I fail to see why this should be the case. Note that
having the possibility to do dlopen() calls in the frontend (aka
libpq) for loading some callbacks is something I've been looking for
in the past. Having consistency in terms of restrictions between
library and command like for archives would make sense I guess (FWIW,
I mentioned on the thread of d627ce3 that we'd better not put any
restrictions for the archives).
--
Michael