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.