Re: pg_stat_replication log positions vs base backups
| От | Michael Paquier |
|---|---|
| Тема | Re: pg_stat_replication log positions vs base backups |
| Дата | |
| Msg-id | CAB7nPqQ-VkZWUOkdNRxo7FBZ7zqo0WuNF9ty2aYiOYsQLVGixw@mail.gmail.com обсуждение исходный текст |
| Ответ на | Re: pg_stat_replication log positions vs base backups (Magnus Hagander <magnus@hagander.net>) |
| Ответы |
Re: pg_stat_replication log positions vs base backups
|
| Список | pgsql-hackers |
On Wed, Nov 25, 2015 at 11:08 PM, Magnus Hagander <magnus@hagander.net> wrote:
On Wed, Nov 25, 2015 at 3:03 PM, Michael Paquier <michael.paquier@gmail.com> wrote:On Wed, Nov 25, 2015 at 7:18 PM, Magnus Hagander <magnus@hagander.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
http://www.postgresql.org/message-id/CAB7nPqRUaFyTJ024W-HiihW4PwfcadZN9HFMsJQfOZSQtZtDUA@mail.gmail.com
Regards,
--
Michael
Вложения
В списке pgsql-hackers по дате отправления: