Re: Patch to improve performance of replay of AccessExclusiveLock

On 16 March 2017 at 09:52, David Rowley <david(dot)rowley(at)2ndquadrant(dot)com> wrote:

> I was just trying to verify if it was going to do the correct thing in
> regards to subtransactions and I got a little confused at the comments at
> the top of StandbyAcquireAccessExclusiveLock():
>
> * We record the lock against the top-level xid, rather than individual
> * subtransaction xids. This means AccessExclusiveLocks held by aborted
> * subtransactions are not released as early as possible on standbys.
>
> if this is true, and it seems to be per LogAccessExclusiveLock(), does
> StandbyReleaseLockTree() need to release the locks for sub transactions too?
>
> This code:
>
> for (i = 0; i < nsubxids; i++)
> StandbyReleaseLocks(subxids[i]);

Yes, you are correct, good catch. It's not broken, but the code does
nothing, slowly.

We have two choices... remove this code or assign locks against subxids.

1. In xact_redo_abort() we can just pass NULL to
StandbyReleaseLockTree(), keeping the other code as is for later
fixing by another approach.

2. In LogAccessExclusiveLock() we can use GetCurrentTransactionId()
rather than GetTopTransactionId(), so that we assign the lock to the
subxid rather than the top xid. That could increase lock traffic, but
less likely. It also solves the problem of early release when AELs
held by subxids.

(2) looks safe enough, so patch attached.

--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services