On 2014-08-11 16:20:41 +0900, Fujii Masao wrote:
> On Mon, Aug 11, 2014 at 3:54 PM, Andres Freund <andres@2ndquadrant.com> wrote:
> > On 2014-08-11 12:42:06 +0900, Fujii Masao wrote:
> >> On Mon, Aug 11, 2014 at 10:48 AM, Tatsuo Ishii <ishii@postgresql.org> wrote:
> >> >> On Mon, Aug 11, 2014 at 9:23 AM, Tatsuo Ishii <ishii@postgresql.org> wrote:
> >> >>> We can know the LSN of last committed WAL record on primary by using
> >> >>> pg_current_xlog_location(). It seems there's no API to know the time
> >> >>> when the WAL record was created. I would like to know standby delay by
> >> >>> using pg_last_xact_replay_timestamp() and such that API.
> >> >>>
> >> >>> If there's no such a API, it would be useful to invent usch an API IMO.
> >> >>
> >> >> +1
> >> >>
> >> >> I proposed that function before, but unfortunately it failed to be applied.
> >> >> But I still think that function is useful to calculate the replication delay.
> >> >> The past discussion is
> >> >> http://www.postgresql.org/message-id/CAHGQGwF3ZjfuNEj5ka683KU5rQUBtSWtqFq7g1X0g34o+JXWBw@mail.gmail.com
> >> >
> >> > I looked into the thread briefly and found Simon and Robert gave -1
> >> > for this because of performance concern. I'm not sure if it's a actual
> >> > performance penalty or not. Maybe we need to major the penalty?
> >>
> >> I think that the performance penalty is negligible small because the patch
> >> I posted before added only three stores to shared memory per
> >> commit/abort.
> >
> > Uh. It adds another atomic operation (the spinlock) to the commit
> > path. That's surely *not* insignificant. At the very least the
> > concurrency approach needs to be rethought.
>
> No, the patch doesn't add the spinlock at all. What the commit path
> additionally does are
>
> 1. increment the counter in shared memory
> 2. set the timestamp of last commit record to shared memory
> 3. increment the counter in shared memory
>
> There is no extra spinlock.
Ah, I see. There's another patch somewhere down that thread
(CAHGQGwG4xFZjfyzaBn5v__d3qpyNNsGBpH3nAr6p40eLivkW5w@mail.gmail.com). The
patch in the message you linked to *does* use a spinlock though.
> OTOH, when pg_last_xact_insert_timestamp reads the timestamp from
> the shared memory, it checks whether the counter values are the same
> or not before and after reading the timestamp. If they are not the same,
> it tries to read the timesetamp again. This logic is necessary for reading
> the consistent timestamp value there.
Yea, that approach then just touches a cacheline that should already be
local. I doubt that the implementation is correct on some more lenient
platforms (missing write memory barrier), but that's not "your fault".
Greetings,
Andres Freund
-- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training &
Services
