On Tue, Apr 05, 2022 at 11:10:30AM +0900, Michael Paquier wrote:
> In the tests, the only difference between the modes "archive_cli" and
> "archive" is the extra option given to the pg_rewind command, and
> that's a simple redirect to what pg_rewind would choose by default
> anyway. A more elegant solution would be to have an extra option in
> RewindTest::run_pg_rewind(), where any caller of the routine can feed
> extra options to the pg_rewind command. Now, in this case, we are
> not going to lose any coverage if the existing "archive" mode is
> changed so as we move away postgresql.conf from the target data folder
> and just use --config-file by default, no? I would make the choice of
> simplicity, by giving up on "archive_cli" and using
> primary-postgresql.conf.tmp as value for --config-file. There could
> be an argument for using --config-file for all the modes, as well.
The clock is ticking, so I have looked at this patch by myself. I
have finished by tweaking the docs, the code and the tests as of the
attached. One thing that I found annoying is the lack of description
about the fact that this option affects pg_rewind when it internally
starts the target cluster, as of when we retrieve restore_command and
when we enforce crash recovery to have a target cluster with a clean
shutdown state. The tests are heavily simplified, having no impact on
the run-time while improving the coverage for the code paths changed
here.
--
Michael