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
signature.asc
Description: PGP signature