Re: pg_stat_replication log positions vs base backups

On Thu, Nov 26, 2015 at 8:17 AM, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
wrote:

>
>
> 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.
>

I'm only talking about the actual value in pg_stat_replication here, not
what we are using internally. These are two different things of course -
let's keep them separate for now. In pg_stat_replication, we explicitly
check for InvalidXLogRecPtr and then explicitly set the resulting value to
NULL in the SQL return.

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/