Re: PATCH: add "--config-file=" option to pg_rewind

Am 06.04.22 um 14:04 schrieb Michael Paquier:
> 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

Ta! Sorry, been ridiculously busy these days, and I do confess that I've
been struggling a bit with those tests before ;-)

> 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

LGTM!

Thx again!
-- 
Gunnar "Nick" Bluth

Eimermacherweg 106
D-48159 Münster

Mobil +49 172 8853339
Email: gunnar.bluth@pro-open.de
__________________________________________________________________________
"Ceterum censeo SystemD esse delendam" - Cato