On 7 March 2017 at 10:01, David Rowley <david.rowley@2ndquadrant.com> wrote:
> On 2 March 2017 at 16:06, Amit Kapila <amit.kapila16@gmail.com> wrote:
>> On Wed, Mar 1, 2017 at 5:32 PM, David Rowley
>> <david.rowley@2ndquadrant.com> wrote:
>>> Hackers,
>>>
>>> I've attached a small patch which aims to improve the performance of
>>> AccessExclusiveLock when there are many AccessExclusiveLock stored.
Looks useful.
> That may need tweaking. Likely it could be smaller if we had some sort
> of bloom filter to mark if the transaction had obtained any AEL locks,
> that way it could skip. Initially I really didn't want to make the
> patch too complex. I had thought that a fairly large hash table would
> fix the problem well enough, as quite possibly most buckets would be
> empty and non empty buckets have short lists.
ISTM that we should mark each COMMIT if it has an AEL, so we can avoid
any overhead in the common case.
So an additional sub-record for the commit/abort wal record, via
include/access/xact.h
>> 4.
>> In some cases, it might slow down the performance where you have to
>> traverse the complete hash table like StandbyReleaseOldLocks, however,
>> I think those cases will be less relative to other lock release calls
>> which are called at every commit, so probably this is okay.
>
> Yes, that may be true. Perhaps making the table smaller would help
> bring that back to what it was, or perhaps the patch needs rethought
> to use bloom filters to help identify if we need to release any locks.
>
> Opinions on that would be welcome. Meanwhile I'll go off and play with
> bloom filters to see if I can knock some percentage points of the perf
> report for StandbyReleaseLocks().
Didn't look at the code closely, but if the common case COMMITs don't
scan the list that would be most of the problem solved. If we did need
a hash table, we should just use a normal hash table?
--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services