Re: [REVIEW] pg_last_xact_insert_timestamp

On Sun, Oct 2, 2011 at 8:19 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> 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.

I agree with Tom here. It seems to be safer to follow the protocol even if
that's not required for now.

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

Not only "control" but also "WAL" might be confusing. What about
"transaction information functions"?

BTW, pg_current_xlog_location() and pg_current_xlog_insert_location()
use the same HINT message as I used for pg_last_xact_insert_timestamp(),
but they are also not WAL *control* function. And, in the document,
they are categorized as "Backup Control Functions", but which sounds also
strange. We should call them "WAL information functions" in both
HINT message and the document?

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

Agreed. I will change the description per your suggestion.

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

Agreed. Will change.

Regards,

--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center