Re: pg_stat_replication log positions vs base backups

On Wed, Nov 25, 2015 at 11:08 PM, Magnus Hagander <magnus(at)hagander(dot)net>
wrote:

> On Wed, Nov 25, 2015 at 3:03 PM, Michael Paquier <
> michael(dot)paquier(at)gmail(dot)com> wrote:
>
>> On Wed, Nov 25, 2015 at 7:18 PM, Magnus Hagander <magnus(at)hagander(dot)net>
>> wrote:
>>
> In particular, it seems that in InitWalSenderSlot, we only initialize the
>>> sent location. Perhaps this is needed?
>>>
>>
>> Yes, that would be nice to start with a clean state. At the same time, I
>> am noticing that pg_stat_get_wal_senders() is comparing flush, apply and
>> write directly with 0. I think those should be InvalidXLogRecPtr. Combined
>> with your patch it gives the attached.
>>
>
> Good point.
>
> Another thought - why do we leave 0/0 in sent location, but turn the other
> three fields to NULL if it's invalid? That seems inconsistent. Is there a
> reason for that, or should we fix that as well while we're at it?
>

In some code paths, values are expected to be equal to 0 as XLogRecPtr
values are doing direct comparisons with each other, so you can think that
0 for a XLogRecPtr is slightly different from InvalidXLogRecPtr that could
be expected to be invalid but *not* minimal, imagine for example that we
decide at some point to redefine it as INT64_MAX. See for example
receivedUpto in xlog.c or WalSndWaitForWal in walsender.c. Honestly, I
think that it would be nice to make all the logic consistent under a unique
InvalidXLogRecPtr banner that is defined at 0 once and for all, adding at
the same time a comment in xlogdefs.h mentioning that this should not be
redefined to a higher value because comparison logic of XlogRecPtr's depend
on that. We have actually a similar constraint with TimeLineID = 0 that is
equivalent to infinite. Patch attached following those lines, which should
be backpatched for correctness.

Btw, I found at the same time a portion of NOT_USED code setting up a
XLogRecPtr incorrectly in GetOldestWALSendPointer.

FWIW, the last time I poked at this code, introducing some kind of
MinimalXLogRecPtr logic different to distinguish from InvalidXLogRecPtr I
hit a wall, others did not like that much:
http://www.postgresql.org/message-id/CAB7nPqRUaFyTJ024W-HiihW4PwfcadZN9HFMsJQfOZSQtZtDUA@mail.gmail.com

Regards,
--
Michael