Horton

Re: Two pg_rewind patches (auto generate recovery conf and ensure clean shutdown)

Alexey Kondratov <a.kondratov@postgrespro.ru>

View thread View on postgr.es
From: a.kondratov@postgrespro.ru
To: Laurenz Albe <laurenz.albe@cybertec.at>, Alvaro Herrera <alvherre@2ndquadrant.com>
Cc: Paul Guo <pguo@pivotal.io>, Michael Paquier <michael@paquier.xyz>, Thomas Munro <thomas.munro@gmail.com>, pgsql-hackers <pgsql-hackers@postgresql.org>, Jimmy Yih <jyih@pivotal.io>, Ashwin Agrawal <aagrawal@pivotal.io>
Date: 2019-09-25T20:22:35Z
Lists: pgsql-hackers 
On 2019-09-25 20:48, Alvaro Herrera wrote:
> CC Alexey for reasons that become clear below.
> 
> Another thing in 0002 is that you're adding a "-R" switch to pg_rewind,
> but we have another patch in the commitfest using the same switch for a
> different purpose.  Maybe you guys need to get to an agreement over who
> uses the letter :-)
> 

Thank you for mentioning me. I've been monitoring silently this thread 
and was ready to modify my patch if this one will proceed faster. It 
seems like it's time :)

On 2019-09-25 22:26, Laurenz Albe wrote:
> 
> I believe that -R should be reserved for creating recovery.conf,
> similar to pg_basebackup.
> 
> Everything else would be confusing.
> 
> I've been missing pg_rewind -R!
> 

Yes, -R is already used in pg_basebackup for the same functionality, so 
it seems natural to use it here as well for consistency.

I will review options naming in my own patch and update it accordingly. 
Maybe -w/-W or -a/-A options will be good, since it's about WALs 
retrieval from archive.


Regards
--
Alexey

P.S. Just noticed that in v12 fullname of -R option in pg_basebackup is 
still --write-recovery-conf, which is good for a backward compatibility, 
but looks a little bit awkward, since recovery.conf doesn't exist 
already, doesn't it? However, one may read it as 
'write-recovery-configuration', then it seems fine.

Commits

  1. Improve test coverage of pg_rewind

  2. Improve handling and coverage of --no-ensure-shutdown in pg_rewind

  3. Fix issues in pg_rewind with --no-ensure-shutdown/--write-recovery-conf

  4. Fix --dry-run mode of pg_rewind

  5. pg_rewind: test new --write-recovery-conf functionality

  6. pg_rewind: Allow writing recovery configuration

  7. Have pg_rewind run crash recovery before rewinding

  8. Split out recovery confing-writing code from pg_basebackup

  9. Switch TAP tests of pg_rewind to use non-superuser role, take two