On Mon, Sep 01, 2025 at 02:06:34AM +0000, Hayato Kuroda (Fujitsu) wrote:
> WriteRecoveryConfig() has been used to setup the recovery parameters. Can we
> follow the way to restore them?
>
> Also, can we add a test to 040_pg_createsubscriber? IIUC it is enough to check
> one of recovery parameter is reset after the conversion.
Yeah, we'd want some tests to check the behaviors and expectations in
this tool. This tool is complex enough that this is going to be
mandatory, and making a test cheaper is always nicer.
FWIW, I find the proposed patch a bit dangerous. It updates
pg_createsubscriber.c so as an ALTER SYSTEM is used to reset the
parameters, but the recovery parameters are updated via
WriteRecoveryConfig() which is the code path holding the knowledge
that postgresql.auto.conf is used to hold the recovery parameters.
I don't like much the fact this creates a duplication with
setup_recovery() for the list of parameters handled. All the recovery
parameters are forced to a hardcoded value, except
recovery_target_lsn. So perhaps it would be better to maintain in
pg_createsubscriber.c a list made of (GUC names, values), with the LSN
part handled as an exception for the value to assign.
GenerateRecoveryConfig() can work with a replication connection,
relying on ALTER SYSTEM would not be able to do that properly, so
perhaps we should just invent a new routine that resets a portion of
the file on disk because recovery_gen.c's code already assumes that it
has write access to a data folder to do its work?
--
Michael
