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

On Thu, Sep 26, 2019 at 1:48 AM Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
wrote:

> CC Alexey for reasons that become clear below.
>
> On 2019-Sep-25, Paul Guo wrote:
>
> > > Question about 0003. If we specify --skip-clean-shutdown and the
> cluter
> > > was not cleanly shut down, shouldn't we error out instead of trying to
> > > press on?
> >
> > pg_rewind would error out in this case, see sanityChecks().
> > Users are expected to clean up themselves if they use this argument.
>
> Ah, good. We should have a comment about that below the relevant
> stanza, I suggest. (Or maybe in the same comment that ends in line
> 272).
>
> I pushed 0001 with a few tweaks. Nothing really substantial, just my
> OCD that doesn't leave me alone ... but this means your subsequent
> patches need to be adjusted. One thing is that that patch touched
> pg_rewind for no reason (those changes should have been in 0002) --
> dropped those.
>
> 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 :-) Also, it would be super helpful if you review
> Alexey's patch: https://commitfest.postgresql.org/24/1849/
>
>
> This line is far too long:
>
> + printf(_(" -s, --skip-clean-shutdown skip running single-mode
> postgres if needed to make sure target is clean shutdown\n"));
>
> Can we make the description more concise?
>

Thanks. I've updated the reset two patches and attached as v8.

Note in the 2nd patch, the long option is changed as below. Both the option
and description
now seems to be more concise since we want db state as either DB_SHUTDOWNED
or
DB_SHUTDOWNED_IN_RECOVERY.

"-s, --no-ensure-shutdowned do not auto-fix unclean shutdown"