Re: Patch to improve performance of replay of AccessExclusiveLock

On 18 March 2017 at 21:59, Simon Riggs <simon(at)2ndquadrant(dot)com> wrote:
> As Amit says, I don't see the gain from adding that to each xact state.
>
> I'd suggest refactoring my patch so that the existign
> MyXactAccessedTempRel becomes MyXactFlags and we can just set a flag
> in the two cases (temp rels and has-aels). That way we still have one
> Global but it doesn't get any uglier than it already is.

OK, now that I understand things a bit better, I've gone over your
patch and changing things around that it combines the two variables
into a flags variable which will be better suited for future
expansion.

I also fixed up the missing code from the Abort transaction. This is patch 0001

0002:

I was thinking it might be a good idea to back patch the patch which
removes the bogus code from StandbyReleaseLockTree(). Any users of
sub-transactions are currently paying a price that they've no reason
to, especially so when there's AELs held by other concurrent
transactions. We already know this code is slow, having this mistake
in there is not helping any.

0003:

Is intended to be patched atop of 0002 (for master only) and revises
this code further to remove the StandbyReleaseLockTree() function. To
me it seems better to do this to clear up any confusion about what's
going on here. This patch does close the door a little on tracking
AELs on sub-xacts. I really think if we're going to do that, then it
won't be that hard to put it back again. Seems better to be consistent
here and not leave any code around that causes people to be confused
about the same thing as I was confused about. This patch does get rid
of StandbyReleaseLockTree() which isn't static. Are we likely to break
any 3rd party code by doing that?

0004:

Again master only, is a further small optimization and of
StandbyReleaseLocks() to further tighten the slow loop over each held
lock. I also think that it's better to be more explicit about the case
of when the function would release all locks. Although I'm not all
that sure in which case the function will actually be called with an
invalid xid.

Patch series is attached.

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