Re: Two pg_rewind patches (auto generate recovery conf and ensure clean shutdown)

Hi Alvaro,

On 30.09.2019 20:13, Alvaro Herrera wrote:
> OK, I pushed this patch as well as Alexey's test patch. It all works
> for me, and the coverage report shows that we're doing the new thing ...
> though only in the case that rewind *is* required. There is no test to
> verify the case where rewind is *not* required. I guess it'd also be
> good to test the case when we throw the new error, if only for
> completeness ...

I've directly followed your guess and tried to elaborate pg_rewind test
cases and... It seems I've caught a few bugs:

1) --dry-run actually wasn't completely 'dry'. It did update target
controlfile, which could cause repetitive pg_rewind calls to fail after
dry-run ones.

2) --no-ensure-shutdown flag was broken, it simply didn't turn off this
new feature.

3) --write-recovery-conf didn't obey the --dry-run flag.

Thus, it was definitely a good idea to add new tests. Two patches are
attached:

1) First one fixes all the issues above;

2) Second one slightly increases pg_rewind overall code coverage from
74% to 78.6%.

Should I put this fix on the next commitfest?

Regards

--
Alexey Kondratov

Postgres Professional https://www.postgrespro.com
Russian Postgres Company

P.S. My apologies that I've missed two of these bugs during review.