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

From: Alexey Kondratov <a(dot)kondratov(at)postgrespro(dot)ru>
To: Paul Guo <pguo(at)pivotal(dot)io>, 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>
Subject: Re: Two pg_rewind patches (auto generate recovery conf and ensure clean shutdown)
Date: 2019-09-27 12:18:59
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On 27.09.2019 6:27, Paul Guo wrote:
> 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.

I went through the remaining two patches and they seem to be very clear
and concise. However, there are two points I could complain about:

1) Maybe I've missed it somewhere in the thread above, but currently
pg_rewind allows to run itself with -R and --source-pgdata. In that case
-R option is just swallowed and neither standby.signal, nor is written, which is reasonable though. Should it
be stated somehow in the docs that -R option always has to go altogether
with --source-server? Or should pg_rewind notify user that options are
incompatible and no recovery configuration will be written?

2) Are you going to leave -R option completely without tap-tests?
Attached is a small patch, which tests -R option along with the existing
'remote' case. If needed it may be split into two separate cases. First,
it tests that pg_rewind is able to succeed with minimal permissions
according to the Michael's patch d9f543e [1]. Next, it checks presence
of standby.signal and adds REPLICATION permission to rewind_user to test
that new standby is able to start with generated recovery configuration.



Alexey Kondratov

Postgres Professional
Russian Postgres Company

Attachment Content-Type Size
v9-0003-Test-new-standby-start-with-generated-config-duri.patch text/x-patch 2.8 KB

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Alvaro Herrera 2019-09-27 12:33:41 Re: pgbench - allow to create partitioned tables
Previous Message Konstantin Knizhnik 2019-09-27 12:07:18 Re: Built-in connection pooler