Horton

Re: [HACKERS] Moving relation extension locks out of heavyweight lock manager

Masahiko Sawada <sawada.mshk@gmail.com>

View thread
From: Masahiko Sawada <sawada.mshk@gmail.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Robert Haas <robertmhaas@gmail.com>, Thomas Munro <thomas.munro@enterprisedb.com>, Amit Kapila <amit.kapila16@gmail.com>, PostgreSQL-development <pgsql-hackers@postgresql.org>
Date: 2017-11-20T22:19:30Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Allow page lock to conflict among parallel group members.

  2. Allow relation extension lock to conflict among parallel group members.

  3. Add assert to ensure that page locks don't participate in deadlock cycle.

  4. Assert that we don't acquire a heavyweight lock on another object after

  5. Fix unsafe usage of strerror(errno) within ereport().

Attachments

On Tue, Nov 14, 2017 at 4:36 PM, Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> Thank you for pointing out and comments.
>
> On Fri, Nov 10, 2017 at 12:38 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Robert Haas <robertmhaas@gmail.com> writes:
>>> No, that's not right.  Now that you mention it, I realize that tuple
>>> locks can definitely cause deadlocks.  Example:
>>
>> Yeah.  Foreign-key-related tuple locks are another rich source of
>> examples.
>>
>>> ... So I don't
>>> think we can remove speculative insertion locks from the deadlock
>>> detector either.
>>
>> That scares me too.  I think that relation extension can safely
>> be transferred to some lower-level mechanism, because what has to
>> be done while holding the lock is circumscribed and below the level
>> of database operations (which might need other locks).  These other
>> ideas seem a lot riskier.
>>
>> (But see recent conversation where I discouraged Alvaro from holding
>> extension locks across BRIN summarization activity.  We'll need to look
>> and make sure that nobody else has had creative ideas like that.)
>>
>
> It seems that we should focus on transferring only relation extension
> locks as a first step. The page locks would also be safe but it might
> require some fundamental changes related to fast insertion, which is
> discussed on other thread[1]. Also in this case I think it's better to
> focus on relation extension locks so that we can optimize the
> lower-level lock mechanism for it.
>
> So I'll update the patch based on the comment I got from Robert before.
>

Attached updated version patch. I've moved only relation extension
locks out of heavy-weight lock as per discussion so far.

I've done a write-heavy benchmark on my laptop; loading 24kB data to
one table using COPY by 1 client, for 10 seconds. The through-put of
patched is 10% better than current HEAD. The result of 5 times is the
following.

----- PATCHED -----
tps = 178.791515 (excluding connections establishing)
tps = 176.522693 (excluding connections establishing)
tps = 168.705442 (excluding connections establishing)
tps = 158.158009 (excluding connections establishing)
tps = 161.145709 (excluding connections establishing)

----- HEAD -----
tps = 147.079803 (excluding connections establishing)
tps = 149.079540 (excluding connections establishing)
tps = 149.082275 (excluding connections establishing)
tps = 148.255376 (excluding connections establishing)
tps = 145.542552 (excluding connections establishing)

Also I've done a micro-benchmark; calling LockRelationForExtension and
UnlockRelationForExtension tightly in order to measure the number of
lock/unlock cycles per second. The result is,
PATCHED = 3.95892e+06 (cycles/sec)
HEAD = 1.15284e+06 (cycles/sec)
The patched is 3 times faster than current HEAD.

Attached updated patch and the function I used for micro-benchmark.
Please review it.

Regards,

--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center