On Fri, Sep 29, 2017 at 8:10 PM, chenhj <chjis...@163.com> wrote:

> On 2017-09-30 00:53:31,"chenhj" <chjis...@163.com> wrote:
>
> On 2017-09-29 19:29:40,"Alexander Korotkov" <a.korot...@postgrespro.ru>
> wrote:
>
> On Fri, Sep 29, 2017 at 10:07 AM, chenhj <chjis...@163.com> wrote:
>>
>>
>>
> OK.  That makes sense.  Thank you for the explanation.
>
> I still have some minor comments.
>
>
>>     /*
>> +    * Save the WAL filenames of the divergence and the current WAL insert
>> +    * location of the source server. Later only the WAL files between
>> those
>> +    * would be copied to the target data directory.
>>
>
> Comment is outdated.  We don't save filenames anymore, now we save segment
> numbers.
>
>
>> +    * Note:The later generated WAL files in the source server before the
>> end
>> +    * of the copy of the data files must be made available when the
>> target
>> +    * server is started. This can be done by configuring the target
>> server as
>> +    * a standby of the source server.
>> +    */
>
>
> You miss space after "Note:".  Also, it seems reasonable for me to leave
> empty line before "Note:".
>
> # Setup parameter for WAL reclaim
>
>
> Parameter*s*, because you're setting up multiple of them.
>
> # The accuracy of imodification from pg_ls_waldir() is seconds, so sleep
>> one seconds
>
>
> One second without "s".
>
> Also, please check empty lines in 006_wal_copy.pl to be just empty lines
> without tabs.
>
>
> Thanks for your comments, i had fix above problems.
> And also add several line breaks at long line in 006_wal_copy.pl
> Please check this patch again.
>
>
> Sorry, patch v6 did not remove tabs in two empty lines, please use the new
> one.
>

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.

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

Reply via email to