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

On Thu, Sep 28, 2017 at 10:52 PM, chenhj <chjischj(at)163(dot)com> wrote:

> On 2017-09-29 00:43:18，"Alexander Korotkov" <a(dot)korotkov(at)postgrespro(dot)ru>
> wrote:
>
> On Thu, Sep 28, 2017 at 6:44 PM, chenhj <chjischj(at)163(dot)com> wrote:
>
>> On 2017-09-28 01:29:29，"Alexander Korotkov" <a(dot)korotkov(at)postgrespro(dot)ru>
>> wrote:
>>
>> It appears that your patch conflicts with fc49e24f. Please, rebase it.
>>
>>
>> Yes, i had rebased it, Please check the new patch.
>>
>
> Good, now it applies cleanly.
>
> else if (strncmp(path, XLOGDIR"/", strlen(XLOGDIR"/")) == 0 &&
>> IsXLogFileName(path + strlen(XLOGDIR"/")) &&
>> (strcmp(path + strlen(XLOGDIR"/") + 8, divergence_wal_filename + 8) < 0
>> ||
>> strcmp(path + strlen(XLOGDIR"/") + 8, last_source_wal_filename + 8) >
>> 0))
>
>
> According to our conding style, you should leave a space betwen XLOGDIF
> and "/".
> Also, you do a trick by comparison xlog segment numbers using strcmp().
> It's nice, but I would prefer seeing XLogFromFileName() here. It would
> improve code readability and be less error prone during further
> modifications.
>
>
> Thanks for advice!
> I had modified it.
>

OK. Patch becomes better.
I also have more general question. Why do we need upper bound for segment
number (last_source_segno)? I understand the purpose of lower bound
(divergence_segno) which save us from copying extra WAL files, but what is
upper bound for? As I understood, we anyway need to replay most recent WAL
records to reach consistent state after pg_rewind. I propose to
remove last_source_segno unless I'm missing something.

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