Re: Moving relation extension locks out of heavyweight lock manager

From: Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Moving relation extension locks out of heavyweight lock manager
Date: 2017-06-21 15:03:51
Message-ID: CAD21AoAmdW7eWKi28FkXXd_4fjSdzVDpeH1xYVX7qx=SyqYyJA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, May 19, 2017 at 11:12 AM, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
> On Wed, May 17, 2017 at 1:30 AM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>> On Sat, May 13, 2017 at 7:27 AM, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>>> On Fri, May 12, 2017 at 9:14 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>>> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>>>>> On Wed, May 10, 2017 at 8:39 PM, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
>>>>>> ... I'd like to propose to change relation
>>>>>> extension lock management so that it works using LWLock instead.
>>>>
>>>>> That's not a good idea because it'll make the code that executes while
>>>>> holding that lock noninterruptible.
>>>>
>>>> Is that really a problem? We typically only hold it over one kernel call,
>>>> which ought to be noninterruptible anyway.
>>>
>>> During parallel bulk load operations, I think we hold it over multiple
>>> kernel calls.
>>
>> We do. Also, RelationGetNumberOfBlocks() is not necessarily only one
>> kernel call, no? Nor is vm_extend.
>
> Yeah, these functions could call more than one kernel calls while
> holding extension lock.
>
>> Also, it's not just the backend doing the filesystem operation that's
>> non-interruptible, but also any waiters, right?
>>
>> Maybe this isn't a big problem, but it does seem to be that it would
>> be better to avoid it if we can.
>>
>
> I agree to change it to be interruptible for more safety.
>

Attached updated version patch. To use the lock mechanism similar to
LWLock but interrupt-able, I introduced new lock manager for extension
lock. A lot of code especially locking and unlocking, is inspired by
LWLock but it uses the condition variables to wait for acquiring lock.
Other part is not changed from previous patch. This is still a PoC
patch, lacks documentation. The following is the measurement result
with test script same as I used before.

* Copy test script
HEAD Patched
4 436.6 436.1
8 561.8 561.8
16 580.7 579.4
32 588.5 597.0
64 596.1 599.0

* Insert test script
HEAD Patched
4 156.5 156.0
8 167.0 167.9
16 176.2 175.6
32 181.1 181.0
64 181.5 183.0

Since I replaced heavyweight lock with lightweight lock I expected the
performance slightly improves from HEAD but it was almost same result.
I'll continue to look at more detail.

Regards,

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

Attachment Content-Type Size
Moving_extension_lock_out_of_heavyweight_lock_v2.patch application/octet-stream 36.9 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2017-06-21 15:04:34 Re: pg_terminate_backend can terminate background workers and autovacuum launchers
Previous Message Robert Haas 2017-06-21 14:52:32 Re: Useless code in ExecInitModifyTable