Re: [PATCH]make pg_rewind to not copy useless WAL files

From: Alexander Korotkov <a(dot)korotkov(at)postgrespro(dot)ru>
To: chenhj <chjischj(at)163(dot)com>
Cc: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [PATCH]make pg_rewind to not copy useless WAL files
Date: 2017-10-02 15:24:30
Message-ID: CAPpHfdudWQ21YJBYHmXb=bnkghW6jD08tVtHZjU4s396EteoRw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Sun, Oct 1, 2017 at 8:27 PM, chenhj <chjischj(at)163(dot)com> wrote:

> On 2017-10-01 04:09:19,"Alexander Korotkov" <a(dot)korotkov(at)postgrespro(dot)ru>
> wrote:
>
> On Sat, Sep 30, 2017 at 8:18 PM, chenhj <chjischj(at)163(dot)com> wrote:
>
>> On 2017-09-30 02:17:54,"Alexander Korotkov" <a(dot)korotkov(at)postgrespro(dot)ru
>> > wrote:
>>
>>
>> Great. Now code of this patch looks good for me.
>> However, we forgot about documentation.
>>
>> <para>
>>> The result is equivalent to replacing the target data directory with
>>> the
>>> source one. Only changed blocks from relation files are copied;
>>> all other files are copied in full, including configuration files. The
>>> advantage of <application>pg_rewind</> over taking a new base backup,
>>> or
>>> tools like <application>rsync</>, is that <application>pg_rewind</>
>>> does
>>> not require reading through unchanged blocks in the cluster. This
>>> makes
>>> it a lot faster when the database is large and only a small
>>> fraction of blocks differ between the clusters.
>>> </para>
>>
>>
>> At least, this paragraph need to be adjusted, because it states whose
>> files are copied. And probably latter paragraphs whose state about WAL
>> files.
>>
>>
>>
>> Your are rigth.
>> I wrote a draft as following, but i'm afraid whether the english
>> statement is accurate.
>>
>
> I'm not native english speaker too :(
>
> Only the WAL files between the point of divergence and the current WAL
>> insert location of the source server are copied, *for* other WAL files are
>> useless for the target server.
>
>
> I'm not sure about this usage of word *for*. For me, it probably should
> be just removed. Rest of changes looks good for me. Please, integrate
> them into the patch.
>
>
> I had removed the *for* , Pleae check the new patch again.
>

Now, this patch looks good for me. It applies cleanly, builds cleanly,
passes regression tests, new functionality is covered by regression tests.
Code is OK for me and docs too.

I'm marking this patch as "Ready for committer". BTW, authors field in the
commitfest app is empty (https://commitfest.postgresql.org/15/1302/).
Please, put your name there.

I hope this patch will be committed during 2017-11 commitfest. Be ready to
rebase this patch if needed. Thank you for your work.

------
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Konstantin Knizhnik 2017-10-02 15:47:48 Issues with logical replication
Previous Message Nico Williams 2017-10-02 15:23:41 Re: generated columns