Re: Patch to improve performance of replay of AccessExclusiveLock

From: Simon Riggs <simon(at)2ndquadrant(dot)com>
To: David Rowley <david(dot)rowley(at)2ndquadrant(dot)com>
Cc: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Patch to improve performance of replay of AccessExclusiveLock
Date: 2017-03-07 10:20:00
Message-ID: CANP8+jL726uF3fccKv3iik5drODUMrtY0O0DA9f+Bqr3j+h+UA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 7 March 2017 at 10:01, David Rowley <david(dot)rowley(at)2ndquadrant(dot)com> wrote:
> On 2 March 2017 at 16:06, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>> On Wed, Mar 1, 2017 at 5:32 PM, David Rowley
>> <david(dot)rowley(at)2ndquadrant(dot)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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Langote 2017-03-07 10:54:48 Re: exposing wait events for non-backends (was: Tracking wait event for latches)
Previous Message Heikki Linnakangas 2017-03-07 10:18:11 Re: SCRAM authentication, take three