From: | Shubham Barai <shubhambaraiss(at)gmail(dot)com> |
---|---|
To: | Alexander Korotkov <a(dot)korotkov(at)postgrespro(dot)ru> |
Cc: | Andrew Borodin <amborodin86(at)gmail(dot)com>, Kevin Grittner <kgrittn(at)gmail(dot)com>, Heikki Linnakangas <hlinnaka(at)iki(dot)fi>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: GSoC 2017 : Patch for predicate locking in Gist index |
Date: | 2017-10-02 13:12:30 |
Message-ID: | CALxAEPvAo_MH7RnxMoV-x5AMFu2OOf1dToR9R+0ksM8jkH95dw@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Sep 28, 2017 4:30 PM, "Alexander Korotkov" <a(dot)korotkov(at)postgrespro(dot)ru>
wrote:
Hi!
On Wed, Jun 21, 2017 at 10:52 AM, Shubham Barai <shubhambaraiss(at)gmail(dot)com>
wrote:
> Hi,
>
> On 21 June 2017 at 13:11, Heikki Linnakangas <hlinnaka(at)iki(dot)fi> wrote:
>
>> On 06/16/2017 01:24 PM, Shubham Barai wrote:
>>
>>> @@ -497,6 +499,13 @@ gistplacetopage(Relation rel, Size freespace,
>>> GISTSTATE *giststate,
>>> for (ptr = dist->next; ptr; ptr = ptr->next)
>>> UnlockReleaseBuffer(ptr->buffer);
>>> }
>>> +
>>> + for (ptr = dist; ptr; ptr = ptr->next)
>>> + PredicateLockPageSplit(rel,
>>> +
>>> BufferGetBlockNumber(buffer),
>>> +
>>> BufferGetBlockNumber(ptr->buffer));
>>> +
>>> +
>>>
>>
>> I think this new code needs to go before the UnlockReleaseBuffer() calls
>> above. Calling BufferGetBlockNumber() on an already-released buffer is not
>> cool.
>>
>> - Heikki
>>
>> I know that. This is the old version of the patch. I had sent updated
> patch later. Please have a look at updated patch.
>
I took a look on this patch.
In gistdoinsert() you do CheckForSerializableConflictIn() only if page
wasn't exclusively locked before (xlocked is false).
if (!xlocked)
> {
> LockBuffer(stack->buffer, GIST_UNLOCK);
> LockBuffer(stack->buffer, GIST_EXCLUSIVE);
> CheckForSerializableConflictIn(r, NULL, stack->buffer);
> xlocked = true;
However, page might be exclusively locked before. And in this case
CheckForSerializableConflictIn() would be skipped. That happens very
rarely (someone fixes incomplete split before we did), but nevertheless.
I agree with Andrey Borodin's view on locks. I am quoting his message
"if xlocked = true, page was already checked for conflict after setting
exclusive lock on it's buffer. I still do not see any problem here..."
Regards,
Shubham
From | Date | Subject | |
---|---|---|---|
Next Message | Magnus Hagander | 2017-10-02 13:26:13 | Re: 64-bit queryId? |
Previous Message | Robert Haas | 2017-10-02 13:04:30 | Re: CLUSTER command progress monitor |