From: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | Robert Haas <robertmhaas(at)gmail(dot)com>, Jeff Janes <jeff(dot)janes(at)gmail(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Make HeapTupleSatisfiesMVCC more concurrent |
Date: | 2015-08-26 02:45:38 |
Message-ID: | CAA4eK1Jg13dQWCcMjQGAwVTUGiFzojh92nTgCd79LkaqOTF3dg@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Wed, Aug 26, 2015 at 2:21 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> writes:
>
> > I am wondering that is there any harm in calling TransactionIdDidAbort()
> > in slow path before calling SubTransGetTopmostTransaction(), that can
> > also maintain consistency of checks in both the functions?
>
> I think this is probably a bad idea. It adds a pg_clog lookup that we
> would otherwise not do at all, in hopes of avoiding a pg_subtrans lookup.
> It's not exactly clear that that's a win even if we successfully avoid
> the subtrans lookup (which we often would not). And even if it does win,
> that would only happen if the other transaction has aborted, which isn't
> generally the case we prefer to optimize for.
>
I think Alvaro has mentioned the case where it could win, however it can
still
add some penality where most (or all) transactions are committed. I agree
with
you that we might not want to optimize or spend our energy figuring out if
this
is win for not-a-common use case. OTOH, I feel having this and other point
related to optimisation (one-XID-cache) could be added as part of function
level
comments to help, if some body encounters any such case in future or is
puzzled why there are some differences in TransactionIdIsInProgress() and
XidInMVCCSnapshot().
> I don't mean to dismiss the potential for further optimization inside
> XidInMVCCSnapshot (for instance, the one-XID-cache idea sounds promising);
> but I think that's material for further research and a separate patch.
>
> It's not clear to me if anyone wanted to do further review/testing of
> this patch, or if I should go ahead and push it (after fixing whatever
> comments need to be fixed).
>
I think jeff's test results upthread already ensured that this patch is of
value and fair enough number of people have already looked into it and
provided there feedback, so +1 for pushing it.
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
From | Date | Subject | |
---|---|---|---|
Next Message | Michael Paquier | 2015-08-26 03:24:27 | Re: [PATCH] Reload SSL certificates on SIGHUP |
Previous Message | Jan de Visser | 2015-08-26 02:10:22 | Re: Idea: closing the loop for "pg_ctl reload" |