Re: [REVIEW] pg_last_xact_insert_timestamp

On Thu, Sep 15, 2011 at 4:52 AM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
> Okay, I've changed the patch in that way.

It occurs to me that pgstat_report_xact_end_timestamp doesn't really
need to follow the protocol of bumping the change count before and
after bumping the timestamp. We elsewhere assume that four-byte reads
and writes are atomic, so there's no harm in assuming the same thing
here (and if they're not... then the change-count thing is pretty
dubious anyway). I think it's sufficient to just set the value, full
stop.

Also, in pg_last_xact_insert_timestamp, the errhint() seems a little
strange - this is not exactly a WAL *control* function, is it?

In the documentation, for the short description of
pg_last_xact_insert_timestamp(), how about something like "returns the
time at which a transaction commit or transaction about record was
last inserted into the transaction log"? Or maybe that's too long.
But the current description doesn't seem to do much other than
recapitulate the function name, so I'm wondering if we can do any
better than that.

I think that instead of hacking up the backend-status copying code to
have a mode where it copies everything, you should just have a
special-purpose function that computes the value you need directly off
the backend status entries themselves. This approach seems like it
both clutters the code and adds lots of extra data copying.

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