Re: TransactionIdIsInProgress() cache

On Tue, 2008-03-11 at 12:57 +0000, Heikki Linnakangas wrote:

> We could do this instead:
>
> > if (TransactionIdDidCommit(xvac))
> > {
> > /* committed */
> > }
> > else if (!TransactionIdIsInProgress(xvac))
> > {
> > if (TransactionIdDidCommit(xvac))
> > {
> > /* committed */
> > }
> > else
> > {
> > /* aborted */
> > }
> > }
> > else
> > {
> > /* in-progress */
> > }
>
> (hopefully there would be a way to macroize that or something to avoid
> bloating the code any more.)
>
> For committed transactions, this would save the
> TransactionIdIsInProgress call completely, whether or not it's in the
> one-item cache. The tradeoff is that we would have to call
> TransactionIdDidCommit twice for aborted transactions.

I thought about doing it the way you suggest but
TransactionIdIsInProgress is the offending code, so thats the part I
tuned. TransactionIdDidCommit uses the single item cache so it seemed
easier to make isolated changes as proposed rather than touching tqual
code in multiple places to do exactly the same thing.

No, I haven't done any formal performance testing on it. It seemed an
obvious hole that everyone would agree we should avoid, since we can do
it so cheaply: one integer comparison against scanning the whole
procarray and taking an LWlock.

--
Simon Riggs
2ndQuadrant http://www.2ndQuadrant.com

PostgreSQL UK 2008 Conference: http://www.postgresql.org.uk