On Tue, Mar 19, 2019 at 02:09:03PM +0800, Paul Guo wrote:
> The first patch adds an option to automatically generate recovery conf
> contents in related files, following pg_basebackup. In the patch,
> GenerateRecoveryConf(), WriteRecoveryConf() and escape_quotes() are almost
> same as them on pg_basebackup. The main difference is due to replication
> slot support and code (variables) limit. It seems that we could slightly
> refactor later to put some common code into another file after aligning
> pg_rewind with pg_basebackup. This was tested manually and was done by
> Jimmy (cc-ed), Ashiwin (cc-ed) and me.
Interesting. The two routines have really the same logic, I would
recommend to have a first patch which does the refactoring and have
pg_rewind use it, and then a second patch which writes recovery.conf
and uses the first patch to get the contents. Please note that the
This is a good suggestion also. Will do it.
common routine needs to be version-aware as pg_basebackup requires
compatibility with past versions, but you could just pass the version
number from the connection, and have pg_rewind pass the compiled-in
version value.
> Another patch does automatic clean shutdown by running a single mode
> postgres instance if the target was not clean shut down since that is
> required by pg_rewind. This was manually tested and was done by Jimmy
> (cc-ed) and me. I'm not sure if we want a test case for that though.
I am not sure that I see the value in that. I'd rather let the
required service start and stop out of pg_rewind and not introduce
dependencies with other binaries. This step can also take quite some
This makes recovery more automatically. Yes, it will add the dependency on the postgres
binary, but it seems that most time pg_rewind should be shipped as postgres
in the same install directory. From my experience of manually testing pg_rewind,
I feel that this besides auto-recovery-conf writing really alleviate my burden. I'm not sure how
other users usually do before running pg_rewind when the target is not cleanly shut down,
but probably we can add an argument to pg_rewind to give those people who want to
handle target separately another chance? default on or off whatever.
time depending on the amount of WAL to replay post-crash at recovery
and the shutdown checkpoint which is required to reach a consistent
on-disk state.
The time is still required for people who want to make the target ready for pg_rewind in another way.
Thanks.