Re: GSoC 2017 : Patch for predicate locking in Gist index

Hi!

On Wed, Jun 21, 2017 at 10:52 AM, Shubham Barai <shubhambaraiss@gmail.com>
wrote:

> Hi,
>
> On 21 June 2017 at 13:11, Heikki Linnakangas <hlinnaka@iki.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.

------
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company