pg_last_xact_insert_timestamp

On Thu, Sep 8, 2011 at 9:36 AM, Fujii Masao <masao.fujii@gmail.com> wrote:

On Thu, Sep 8, 2011 at 7:06 AM, Chris Redekop <chris@replicon.com> wrote:

Is there anything available to get the last time a transaction
occurred?....like say "pg_last_xact_timestamp"?  In order to accurately
calculate how far behind my slave is I need to do something like
master::pg_last_xact_timestamp() -
slave::pg_last_xact_replay_timestamp()....currently I'm using now() instead
of the pg_last_xact_timestamp() call, but then when the master is not busy
the slave appears to lag behind.  I'm considering writing a C module to get
the last modified file time of the xlog, but I'm hoping there is a better
alternative that I haven't found yet....

The above has been posted in pgsql-general. The reason why Chris thinks
a counterpart of pg_last_xact_replay_timestamp() is required sounds
reasonable to me. So I'd like to propose new function
"pg_last_xact_insert_timestamp" as the counterpart, which returns the
timestamp of the last inserted commit or abort WAL record. I'll add the
attached patch into the next CF.

For reasons stated on the original thread, I don't think we need this.

The OP can calculate what he wants without this.

The code already exists in recovery and has some purpose there. Adding
similar code to the mainline just so somebody can run this function
occasionally is not good. Based on this I will be looking to see if we
can optimise the recovery path some more.

--
 Simon Riggs                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

On Thu, Sep 8, 2011 at 5:55 PM, Simon Riggs <simon@2ndquadrant.com> wrote:

On Thu, Sep 8, 2011 at 9:36 AM, Fujii Masao <masao.fujii@gmail.com> wrote:

The above has been posted in pgsql-general. The reason why Chris thinks
a counterpart of pg_last_xact_replay_timestamp() is required sounds
reasonable to me. So I'd like to propose new function
"pg_last_xact_insert_timestamp" as the counterpart, which returns the
timestamp of the last inserted commit or abort WAL record. I'll add the
attached patch into the next CF.

For reasons stated on the original thread, I don't think we need this.

The OP can calculate what he wants without this.

The code already exists in recovery and has some purpose there. Adding
similar code to the mainline just so somebody can run this function
occasionally is not good. Based on this I will be looking to see if we
can optimise the recovery path some more.

Okay. Let me explain another use case of pg_last_xact_insert_timestamp().
The existing functions might be enough for checking whether the standby
has already caught up with the master. But I think that new function would be
very useful to calculate *how much* the standby is behind from the master.

Of course, we can do that by using the existing functions. But a problem is
that they return LSN and the calculated delay is represented as the size of
WAL. For users, it's not easy to handle LSN (e.g., there is no function to do
calculation of LSN), and the delay in WAL size is not intuitive. I've sometimes
received such a complaint.

OTOH, new function enables users to monitor the delay as a timestamp.
For users, a timestamp is obviously easier to handle than LSN, and the delay
as a timestamp is more intuitive. So, I think that it's worth adding
something like pg_last_xact_insert_timestamp into core for improvement
of user-friendness.

Regards,

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

On Thu, Sep 8, 2011 at 6:14 AM, Fujii Masao <masao.fujii@gmail.com> wrote:

OTOH, new function enables users to monitor the delay as a timestamp.
For users, a timestamp is obviously easier to handle than LSN, and the delay
as a timestamp is more intuitive. So, I think that it's worth adding
something like pg_last_xact_insert_timestamp into core for improvement
of user-friendness.

It seems very nice from a usability point of view, but I have to agree
with Simon's concern about performance. Actually, as of today,
WALInsertLock is such a gigantic bottleneck that I suspect the
overhead of this additional bookkeeping would be completely
unnoticeable. But I'm still reluctant to add more centralized
spinlocks that everyone has to fight over, having recently put a lot
of effort into getting rid of some of the ones we've traditionally
had.

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

Thanks for all the feedback guys. Just to throw another monkey wrench in
here - I've been playing with Simon's proposed solution of returning 0 when
the WAL positions match, and I've come to the realizatiion that even if
using pg_last_xact_insert_timestamp, although it would help, we still
wouldn't be able to get a 100% accurate "how far behind?" counter....not
that this is a big deal, but I know my ops team is going to bitch to me
about it :).....take this situation: there's a lull of 30 seconds where
there are no transactions committed on the server....the slave is totally
caught up, WAL positions match, I'm reporting 0, everything is happy. Then
a transaction is committed on the master....before the slave gets it my
query hits it and sees that we're 30 seconds behind (when in reality we're
<1sec behind).....Because of this affect my graph is a little spikey...I
mean it's not a huge deal or anything - I can put some sanity checking in my
number reporting ("if 1 second ago you were 0 seconds behind, you can't be
more than 1 second behind now" sorta thing). But if we wanted to go for
super-ideal solution there would be a way to get the timestamp of
pg_stat_replication.replay_location+1 (the first transaction that the slave
does not have).

On Thu, Sep 8, 2011 at 7:03 AM, Robert Haas <robertmhaas@gmail.com> wrote:

On Thu, Sep 8, 2011 at 10:03 PM, Robert Haas <robertmhaas@gmail.com> wrote:

On Thu, Sep 8, 2011 at 6:14 AM, Fujii Masao <masao.fujii@gmail.com> wrote:

OTOH, new function enables users to monitor the delay as a timestamp.
For users, a timestamp is obviously easier to handle than LSN, and the delay
as a timestamp is more intuitive. So, I think that it's worth adding
something like pg_last_xact_insert_timestamp into core for improvement
of user-friendness.

It seems very nice from a usability point of view, but I have to agree
with Simon's concern about performance.  Actually, as of today,
WALInsertLock is such a gigantic bottleneck that I suspect the
overhead of this additional bookkeeping would be completely
unnoticeable.

The patch I've posted adds one acquisition and release of spinlock per
a commit or abort. But it's done outside of WALInsertLock. So I don't
think that the patch degrades a performance.

 But I'm still reluctant to add more centralized
spinlocks that everyone has to fight over, having recently put a lot
of effort into getting rid of some of the ones we've traditionally
had.

You are against adding new acquisition and release of spinlock itself
even if it has nothing to do with WALInsertLock? The patch uses
XLogCtl->info_lck spinlock to save the last insert timestamp in the
shmem. XLogCtl->info_lck already protects many shmem variables
related to XLOG. So using XLogCtl->info_lck additionally might make
its contention heavier and degrade a performance. If the patch
defines new spinlock and uses it to save the timestamp to avoid
such a contention, you feel satisfied with the patch?

Another idea to avoid spinlock contention is save the timestamp in
PgBackendStatus (which contains information for pg_stat_activity).
This enables us to write and read the timestamp without spinlock.
Comments?

Regards,

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

On Thu, Sep 8, 2011 at 8:42 PM, Fujii Masao <masao.fujii@gmail.com> wrote:

Another idea to avoid spinlock contention is save the timestamp in
PgBackendStatus (which contains information for pg_stat_activity).
This enables us to write and read the timestamp without spinlock.
Comments?

That seems like a possibly promising approach, in that each backend
could update the information separately, and it's the reader's job to
go find the maximum of all those values when needed. So the overhead
is (properly, in this case) placed on the reader instead of the
writer. But it's a bit tricky, because when the reader wants that
maximum, it has to take into account inactive backends that may have
committed transactions before exiting, not just the ones that are
still connected.

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

On Sat, Sep 10, 2011 at 12:32 AM, Robert Haas <robertmhaas@gmail.com> wrote:

On Thu, Sep 8, 2011 at 8:42 PM, Fujii Masao <masao.fujii@gmail.com> wrote:

Another idea to avoid spinlock contention is save the timestamp in
PgBackendStatus (which contains information for pg_stat_activity).
This enables us to write and read the timestamp without spinlock.
Comments?

That seems like a possibly promising approach, in that each backend
could update the information separately, and it's the reader's job to
go find the maximum of all those values when needed.  So the overhead
is (properly, in this case) placed on the reader instead of the
writer.  But it's a bit tricky, because when the reader wants that
maximum, it has to take into account inactive backends that may have
committed transactions before exiting, not just the ones that are
still connected.

Yes, that's what I was thinking. The attached patch has adopted that
approach.

Regards,

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

Hi, This is a review for pg_last_xact_insert_timestamp patch.
(https://commitfest.postgresql.org/action/patch_view?id=634)

Summary
====================
There's one question and two comments.

Q1: The shmem entry for timestamp is not initialized on
allocating. Is this OK? (I don't know that for OSs other than
Linux) And zeroing double field is OK for all OSs?

And you can find the two more comment in "Code details" section.

I have no objection for commiter review with the Q1 above is
cleared.

Purpose and function of this patch
====================

This patch allows users to grip the amount of replay lag on the
standby by giving the timestamp of the WAL latestly inserted on
the primary.

Each backend writes the timestamp of the just inserted log record
of commit/abort freely on PgBackendStatus, and the function
pg_last_xact_insert_timestamp() gathers them for all backends
including inactive ones and returns the latest one, or NULL when
no timestamp is saved.

Implemented in lockless way.

Patch application, regression test
====================
This patch applies cleanly onto HEAD.
make world completed successfully. make check passed.
Style of the code and patch seems correct.

Documentation
====================
It looks properly documented for new functions.

Performance
====================

This patch writes one TimestampTz value on shmem per one WAL
insertion for commit/abort with no lock, and collect the values
for all backends on reading without any locks.

I think the former gives virtually no penalty for performance of
transactions, and the latter has no influence on other
transactions thanks to the lockless implement.

Code details
====================

== Shared memory initialization

beentry->st_xact_end_timestamp is not initialized on backend
startup. This is because the life time of the field is beyond
that of backends. On the other hand, Linux man page says that
newly created shared memory segment is zeroed, but I don't have
information about other OSs.

Nevertheless this is ok for all OSs, I don't know whether
initializing TimestampTz(double, int64 is ok) field with 8 bytes
zeros is OK or not, for all platforms. (It is ok for
IEEE754-binary64).

== Modification detection protocol in pgstat.c

In pgstat_report_xact_end_timestamp, `beentry->st_changecount
protocol' is used. It is for avoiding reading halfway-updated
beentry as described in pgstat.h. Meanwhile,
beentry->st_xact_end_timestamp is not read or (re-)initialized in
pgstat.c and xlog.c reads only this field of whole beentry and
st_changecount is not get cared here..

So I think at present it is needless to touch st_changecount
here.

Of cource the penalty is very close to nothing so it may be there
for future use...

== Code duplication in xact.c

in xact.c, same lines inserted into the end of both IF and ELSE
blocks.

xact.c:1047> pgstat_report_xact_end_timestamp(xlrec.xact_time);
xact.c:1073> pgstat_report_xact_end_timestamp(xlrec.xact_time);

These two lines refer to xlrec.xact_time, both of which comes
from xactStopTimestamp freezed at xact.c:986

xact.c:986> SetCurrentTransactionStopTimestamp();
xact.c:1008> xlrec.xact_time = xactStopTimestamp;
xact.c:1051> xlrec.xact_time = xactStopTimestamp;

I think it is better to move this line to just after this ELSE
block using xactStopTimestamp instead of xlrec.xact_time. Please
leave it as it is if you put more importance on the similarity
with the code inserted into RecordTransactionAbort().

Conclusion
====================

With the issue of shmem zeroing on other OSs is cleard, I have no
objection to commit this patch.

Sincerely,

--
Kyotaro Horiguchi
NTT Open Source Software Center

On Wed, Sep 14, 2011 at 6:21 PM, Kyotaro HORIGUCHI
<horiguchi.kyotaro@oss.ntt.co.jp> wrote:

Hi, This is a review for pg_last_xact_insert_timestamp patch.
(https://commitfest.postgresql.org/action/patch_view?id=634)

Thanks for the review!

Q1: The shmem entry for timestamp is not initialized on
allocating. Is this OK? (I don't know that for OSs other than
Linux) And zeroing double field is OK for all OSs?

CreateSharedBackendStatus() initializes that shmem entries by doing
MemSet(BackendStatusArray, 0, size). You think this is not enough?

Nevertheless this is ok for all OSs, I don't know whether
initializing TimestampTz(double, int64 is ok) field with 8 bytes
zeros is OK or not, for all platforms. (It is ok for
IEEE754-binary64).

Which code are you concerned about?

== Modification detection protocol in pgstat.c

In pgstat_report_xact_end_timestamp, `beentry->st_changecount
protocol' is used. It is for avoiding reading halfway-updated
beentry as described in pgstat.h. Meanwhile,
beentry->st_xact_end_timestamp is not read or (re-)initialized in
pgstat.c and xlog.c reads only this field of whole beentry and
st_changecount is not get cared here..

No, st_changecount is used to read st_xact_end_timestamp.
st_xact_end_timestamp is read from the shmem to the local memory
in pgstat_read_current_status(), and this function always checks
st_changecount when reading the shmem value.

== Code duplication in xact.c

in xact.c, same lines inserted into the end of both IF and ELSE
blocks.

xact.c:1047>    pgstat_report_xact_end_timestamp(xlrec.xact_time);
xact.c:1073>    pgstat_report_xact_end_timestamp(xlrec.xact_time);

These two lines refer to xlrec.xact_time, both of which comes
from xactStopTimestamp freezed at xact.c:986

xact.c:986>     SetCurrentTransactionStopTimestamp();
xact.c:1008>    xlrec.xact_time = xactStopTimestamp;
xact.c:1051>    xlrec.xact_time = xactStopTimestamp;

I think it is better to move this line to just after this ELSE
block using xactStopTimestamp instead of xlrec.xact_time.

Okay, I've changed the patch in that way.

Regards,

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

On Thu, Sep 15, 2011 at 4:52 AM, Fujii Masao <masao.fujii@gmail.com> wrote:

Thanks for the review!

Koyotaro Horiguchi -

Are you going to re-review the latest version of this patch?

Thanks,

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

Sorry for late to re-review.

One question is remaind,

Q1: The shmem entry for timestamp is not initialized on
allocating. Is this OK? (I don't know that for OSs other than
Linux) And zeroing double field is OK for all OSs?

CreateSharedBackendStatus() initializes that shmem entries by doing
MemSet(BackendStatusArray, 0, size). You think this is not enough?

Sorry for missing it. That's enough.

Nevertheless this is ok for all OSs, I don't know whether
initializing TimestampTz(double, int64 is ok) field with 8 bytes
zeros is OK or not, for all platforms. (It is ok for
IEEE754-binary64).

Which code are you concerned about?

xlog.c: 5889

beentry = pgstat_fetch_all_beentry();

for (i = 0; i < MaxBackends; i++, beentry++)
{
xtime = beentry->st_xact_end_timestamp;

I think the last line in quoted code above reads possibly
zero-initialized double (or int64) value, then the doubted will
be compared and copied to another double.

if (result < xtime)
result = xtime;

No, st_changecount is used to read st_xact_end_timestamp.
st_xact_end_timestamp is read from the shmem to the local memory
in pgstat_read_current_status(), and this function always checks
st_changecount when reading the shmem value.

Yes, I confirmed that pg_lats_xact_insert_timestamp looks local
copy of BackendStatus. I've found it not unnecessary.

--
Kyotaro Horiguchi
NTT Open Source Software Center

On Thu, Sep 29, 2011 at 5:20 PM, Kyotaro HORIGUCHI
<horiguchi.kyotaro@oss.ntt.co.jp> wrote:

Sorry for late to re-review.

Thanks!

Nevertheless this is ok for all OSs, I don't know whether
initializing TimestampTz(double, int64 is ok) field with 8 bytes
zeros is OK or not, for all platforms. (It is ok for
IEEE754-binary64).

Which code are you concerned about?

xlog.c: 5889

      beentry = pgstat_fetch_all_beentry();

      for (i = 0; i < MaxBackends; i++, beentry++)
      {
              xtime = beentry->st_xact_end_timestamp;

 I think the last line in quoted code above reads possibly
zero-initialized double (or int64) value, then the doubted will
be compared and copied to another double.

              if (result < xtime)
                      result = xtime;

I believe it's safe. Such a code is placed elsewhere in the source, too.
If it's unsafe, we should have received lots of bug reports related
to that. But we've not.

Regards,

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

Hello,

I understand that it has been at least practically no problem.

Ok, I send this patch to comitters.

Thanks for your dealing with nuisance questions.

At Thu, 29 Sep 2011 21:21:32 +0900, Fujii Masao <masao.fujii@gmail.com> wrote in <CAHGQGwG0C21F0CZY5ExX-49dxdx7hJuNeiBBJ0Tzvh+7vMXWgw@mail.gmail.com>

Nevertheless this is ok for all OSs, I don't know whether
initializing TimestampTz(double, int64 is ok) field with 8 bytes

...

I believe it's safe. Such a code is placed elsewhere in the source, too.
If it's unsafe, we should have received lots of bug reports related
to that. But we've not.

Regards,

--
Kyotaro Horiguchi
NTT Open Source Software Center

On Fri, Sep 30, 2011 at 3:24 AM, Kyotaro HORIGUCHI
<horiguchi.kyotaro@oss.ntt.co.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.

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.

If we want to measure times, we can easily send regular messages into
WAL to provide this function. Using checkpoint records would seem
frequent enough to me. We don't always send checkpoint records but we
can send an info message instead if we are streaming. If
archive_timeout is not set and max_wal_senders > 0 then we can send an
info WAL message with timestamp on a regular cycle.

Alternatively, we can send regular special messages from WALSender to
WALReceiver that do not form part of the WAL stream, so we don't bulk
up WAL archives. (i.e. don't use "w" messages). That seems like the
most viable approach to this problem.

--
 Simon Riggs                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

On Fri, Sep 30, 2011 at 3:18 AM, Simon Riggs <simon@2ndquadrant.com> wrote:

On Fri, Sep 30, 2011 at 3:24 AM, Kyotaro HORIGUCHI
<horiguchi.kyotaro@oss.ntt.co.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

On Fri, Sep 30, 2011 at 8:11 PM, Robert Haas <robertmhaas@gmail.com> wrote:

On Fri, Sep 30, 2011 at 3:18 AM, Simon Riggs <simon@2ndquadrant.com> wrote:

On Fri, Sep 30, 2011 at 3:24 AM, Kyotaro HORIGUCHI
<horiguchi.kyotaro@oss.ntt.co.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.

If the feature could not be done another way, easily, I might agree.

The point is it isn't necessary, useful or elegant to do it this way
and *any* cycles added to mainline transactions need to have careful
justification. And there really isn't any justification for doing
things this way other than its the first way somebody thought of.

--
 Simon Riggs                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

On Fri, Sep 30, 2011 at 3:22 PM, Simon Riggs <simon@2ndquadrant.com> wrote:

If the feature could not be done another way, easily, I might agree.

I don't see that you've offered a reasonable alternative. The
alternative proposals that you proposed don't appear to me to be
solving the same problem. AIUI, the requested feature is to be able
to get, on the master, the timestamp of the last commit or abort, just
as we can already get the timestamp of the last commit or abort
replayed on the standby. Nothing you WAL log and no messages you send
to the standby are going to accomplish that goal.

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

On Fri, Sep 30, 2011 at 8:57 PM, Robert Haas <robertmhaas@gmail.com> wrote:

On Fri, Sep 30, 2011 at 3:22 PM, Simon Riggs <simon@2ndquadrant.com> wrote:

If the feature could not be done another way, easily, I might agree.

I don't see that you've offered a reasonable alternative.  The
alternative proposals that you proposed don't appear to me to be
solving the same problem.  AIUI, the requested feature is to be able
to get, on the master, the timestamp of the last commit or abort, just
as we can already get the timestamp of the last commit or abort
replayed on the standby.  Nothing you WAL log and no messages you send
to the standby are going to accomplish that goal.

The goal of the OP was to find out the replication delay. This
function was suggested, but its not the only way.

My alternative proposals solve the original problem in a better way.

--
 Simon Riggs                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

On Fri, Sep 30, 2011 at 4:07 PM, Simon Riggs <simon@2ndquadrant.com> wrote:

On Fri, Sep 30, 2011 at 8:57 PM, Robert Haas <robertmhaas@gmail.com> wrote:

On Fri, Sep 30, 2011 at 3:22 PM, Simon Riggs <simon@2ndquadrant.com> wrote:

If the feature could not be done another way, easily, I might agree.

I don't see that you've offered a reasonable alternative.  The
alternative proposals that you proposed don't appear to me to be
solving the same problem.  AIUI, the requested feature is to be able
to get, on the master, the timestamp of the last commit or abort, just
as we can already get the timestamp of the last commit or abort
replayed on the standby.  Nothing you WAL log and no messages you send
to the standby are going to accomplish that goal.

The goal of the OP was to find out the replication delay. This
function was suggested, but its not the only way.

My alternative proposals solve the original problem in a better way.

As far as I can see, they don't solve the problem at all. Your
proposals involve sending additional information from the master to
the slave, but the slave already knows both its WAL position and the
timestamp of the transaction it has most recently replayed, because
the startup process on the slave tracks that information and publishes
it in shared memory. On the master, however, only the WAL position is
centrally tracked; the transaction timestamps are not.

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