On Fri, Jul 13, 2018 at 5:40 PM, Kyotaro HORIGUCHI
<horiguchi.kyot...@lab.ntt.co.jp> wrote:
> Hello.
>
> At Wed, 11 Jul 2018 15:09:23 +0900, Masahiko Sawada <sawada.m...@gmail.com>
> wrote in <cad21aocftw6+sn_evtszdajqeeu2ssea2vpcex08ejnafk8...@mail.gmail.com>
>> On Mon, Jul 9, 2018 at 2:47 PM, Kyotaro HORIGUCHI
>> <horiguchi.kyot...@lab.ntt.co.jp> wrote:
> ..
>> Here is review comments of v4 patches.
>>
>> + if (minKeepLSN)
>> + {
>> + XLogRecPtr slotPtr = XLogGetReplicationSlotMinimumLSN();
>> + Assert(!XLogRecPtrIsInvalid(slotPtr));
>> +
>> + tailSeg = GetOldestKeepSegment(currpos, slotPtr);
>> +
>> + XLogSegNoOffsetToRecPtr(tailSeg, 0, *minKeepLSN,
>> wal_segment_size);
>> + }
>>
>> The usage of XLogSegNoOffsetToRecPtr is wrong. Since we specify the
>> destination at 4th argument the wal_segment_size will be changed in
>> the above expression. The regression tests by PostgreSQL Patch Tester
>
> I'm not sure I get this correctly, the definition of the macro is
> as follows.
>
> | #define XLogSegNoOffsetToRecPtr(segno, offset, dest, wal_segsz_bytes) \
> | (dest) = (segno) * (wal_segsz_bytes) + (offset)
>
> The destination is the *3rd* parameter and the forth is segment
> size which is not to be written.
Please see commit a22445ff0b which flipped input and output arguments.
So maybe you need to rebase the patches to current HEAD.
Regards,
--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center
