On Wed, Aug 26, 2015 at 2:21 AM, Tom Lane <
tgl@sss.pgh.pa.us> wrote:
>
> Amit Kapila <
amit.kapila16@gmail.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.
