Re: [REVIEW] pg_last_xact_insert_timestamp

On Fri, Sep 30, 2011 at 3:18 AM, Simon Riggs <simon(at)2ndquadrant(dot)com> wrote:
> On Fri, Sep 30, 2011 at 3:24 AM, Kyotaro HORIGUCHI
> <horiguchi(dot)kyotaro(at)oss(dot)ntt(dot)co(dot)jp> wrote:
>
>> Ok, I send this patch to comitters.
>
> I repeat my objection to this patch. I'm very sorry I haven't been
> around much in last few weeks to keep up a dialogue about this and to
> make it clear how wrong I think this is.
>
> Adding something onto the main path of transaction commit is not good,
> especially when the only purpose of it is to run an occasional
> monitoring query for those people that use replication. Not everybody
> uses replication and even people that do use it don't need to run it
> so frequently that every single commit needs this. This is just bloat
> that we do not need and can also easily avoid.

I think the overhead of this is so completely trivial that we
shouldn't be concerned about it. It's writing 12 bytes to shared
memory for each commit, without taking a lock, in a cache line that
should be minimally contended. We write plenty of other data to
shared memory on each commit, and that's nowhere close to being the
expensive part of committing a transaction. What's expensive is
fighting over WALInsertLock and ProcArrayLock and CLogControlLock, and
possibly waiting for the commit to be durably flushed to disk, and
maybe (at the margin) wrenching the cache lines in our PGPROC away
from whatever processor last stole them to do GetSnapshotData(). I
don't think that a couple of stores to uncontended shared memory are
going to make any difference.

> The calculation itself would be problematic since it differs from the
> way standby_delay is calculated on the standby, which will cause much
> confusion. Some thought or comment should be made about that also.

The string standby_delay doesn't appear in our source tree anywhere,
so I'm not sure what this is referring to. In any case, I'm in favor
of this feature. Currently, the only way to measure the lag on the
standby is to measure it in WAL bytes - and you get to write your own
script to parse the WAL positions. This will allow people to measure
it in minutes. That seems like a significant usability improvement.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company