On Tue, Mar 19, 2019 at 2:18 PM Michael Paquier <mich...@paquier.xyz> wrote:
> 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.
