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-08-16 02:13:41 |
Message-ID: | CAD21AoCKV032aV=Pcu9Mok9ReaBKwWSw6FP2tbA+ib9wGbiv1A@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Thu, Jun 22, 2017 at 12:03 AM, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
> 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.
>
The previous patch conflicts with current HEAD, I rebased the patch to
current HEAD.
Regards,
--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center
Attachment | Content-Type | Size |
---|---|---|
Moving_extension_lock_out_of_heavyweight_lock_v3.patch | application/octet-stream | 37.1 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Alvaro Herrera | 2017-08-16 03:14:04 | Re: Log LDAP "diagnostic messages"? |
Previous Message | Craig Ringer | 2017-08-16 01:37:49 | Re: [PATCH] pageinspect function to decode infomasks |