Re: logical decoding - GetOldestXmin

On 2012-12-13 23:35:00 +0000, Simon Riggs wrote:
> On 13 December 2012 22:37, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
> > On 2012-12-13 17:29:06 -0500, Robert Haas wrote:
> >> On Thu, Dec 13, 2012 at 3:03 PM, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
> >> > It moves a computation of the sort of:
> >> >
> >> > result -= vacuum_defer_cleanup_age;
> >> > if (!TransactionIdIsNormal(result))
> >> > result = FirstNormalTransactionId;
> >> >
> >> > inside ProcArrayLock. But I can't really imagine that to be relevant...
> >>
> >> I can. Go look at some of the 9.2 optimizations around
> >> GetSnapshotData(). Those made a BIG difference under heavy
> >> concurrency and they were definitely micro-optimization. For example,
> >> the introduction of NormalTransactionIdPrecedes() was shockingly
> >> effective.
> >
> > But GetOldestXmin() should be called less frequently than
> > GetSnapshotData() by several orders of magnitudes. I don't really see
> > it being used in any really hot code paths?
>
> Maybe, but that calculation doesn't *need* to be inside the lock, that
> is just a consequence of the current coding.

I am open to suggestion how to do that in a way we a) can hold the lock
already (to safely nail the global xmin to the current value) b) without
duplicating all the code.

Just moving that tidbit inside the lock seems to be the pragmatic
choice. GetOldestXmin is called

* once per checkpoint
* one per index build
* once in analyze
* twice per vacuum
* once for HS feedback messages

Nothing of that occurs frequently enough that 5 instructions will make a
difference. I would be happy to go an alternative path, but right now I
don't see any nice one. A "already_locked" parameter to GetOldestXmin
seems to be a cure worse than the disease.

Greetings,

Andres Freund

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