Re: Two pg_rewind patches (auto generate recovery conf and ensureclean shutdown) - Mailing list pgsql-hackers

From Michael Paquier
Subject Re: Two pg_rewind patches (auto generate recovery conf and ensureclean shutdown)
Date
Msg-id 20190319061851.GB2899@paquier.xyz
Whole thread Raw
In response to Two pg_rewind patches (auto generate recovery conf and ensure clean shutdown)  (Paul Guo <pguo@pivotal.io>)
Responses Re: Two pg_rewind patches (auto generate recovery conf and ensureclean shutdown)
List pgsql-hackers
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
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
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.
--
Michael

Attachment

pgsql-hackers by date:

Previous
From: Yugo Nagata
Date:
Subject: Re: Proposal to suppress errors thrown by to_reg*()
Next
From: Michael Paquier
Date:
Subject: Re: Unaccent extension python script Issue in Windows