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

From: Paul Guo <pguo(at)pivotal(dot)io>
To: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
Cc: Michael Paquier <michael(at)paquier(dot)xyz>, Thomas Munro <thomas(dot)munro(at)gmail(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, Jimmy Yih <jyih(at)pivotal(dot)io>, Ashwin Agrawal <aagrawal(at)pivotal(dot)io>, Alexey Kondratov <a(dot)kondratov(at)postgrespro(dot)ru>
Subject: Re: Two pg_rewind patches (auto generate recovery conf and ensure clean shutdown)
Date: 2019-09-27 03:27:56
Message-ID: CAEET0ZEQZQ1TEDxbTLeu1B_L6Y7G5AYVOs6HH3kSrzGffSMd4Q@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

>
>
> > 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"
>
> Note that "shutdowned" is incorrect English; we've let
> it live in the code because it's not user-visible, but we should
> certainly not immortalize it where it becomes so. I suppose
> "--no-ensure-shutdown" is okay, although I think some may prefer
> "--no-ensure-shut-down". Opinions from native speakers would be
> welcome. Also, let's expand "auto-fix" to "automatically fix" (or
> "repair" if there's room in the line? Not sure. Can be bikeshedded to
> death I guess.)
>

I choose that one from the below tree.

--no-ensure-shutdown
--no-ensure-shutdowned
--no-ensure-clean-shutdown

Now I agree for user experience we should not use the 2nd one. For
--no-ensure-clean-shutdown or -no-ensure-shut-down, seems too many -.

I'm using --no-ensure-shutdown in the new version unless there are better
suggestions.

>
> Secondarily, I see no reason to test connstr_source rather than just
> "conn" in the other patch; doing it the other way is more natural, since
> it's that thing that's tested as an argument.
>
> pg_rewind.c: Please put the new #include line keeping the alphabetical
> order.
>

Agreed to the above suggestions. I attached the v9.

Thanks.

Attachment Content-Type Size
v9-0001-Add-option-to-write-recovery-configuration-inform.patch application/octet-stream 6.0 KB
v9-0002-Ensure-target-clean-shutdown-in-pg_rewind.patch application/octet-stream 6.0 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Youki Shiraishi 2019-09-27 03:29:08 Re: Add comments for a postgres program in bootstrap mode
Previous Message Michael Paquier 2019-09-27 03:23:11 Cleanup code related to OpenSSL <= 0.9.6 in fe/be-secure-openssl.c