Postgres Pro
Downloads
Home   >   mailing lists

Thread: [HACKERS] GSoC 2017: weekly progress reports (week 8)

[HACKERS] GSoC 2017: weekly progress reports (week 8)

From
Shubham Barai
Date:
Project: Explicitly support predicate locks in index AMs besides b-tree

Hi,

During this week, I worked on predicate locking in spgist index. I think, for spgist index, predicate lock only on leaf pages will be enough as spgist searches can determine if there is a match or not only at leaf level.

I have done following things in this week.

1) read the source code of spgist index to understand  the access method

2) found appropriate places to insert calls to existing functions

3) created tests (to verify serialization failures and to demonstrate the feature of reduced false positives) for 'point' and 'box' data types.
    

I will attach the patch shortly.


Regards,
Shubham



Sent with Mailtrack

Re: [HACKERS] GSoC 2017: weekly progress reports (week 8)

From
Shubham Barai
Date:
Hi.

I am attaching a patch for predicate locking in SP-Gist index


Regards,
Shubham



Sent with Mailtrack

On 26 July 2017 at 20:50, Shubham Barai <shubhambaraiss@gmail.com> wrote:
Project: Explicitly support predicate locks in index AMs besides b-tree

Hi,

During this week, I worked on predicate locking in spgist index. I think, for spgist index, predicate lock only on leaf pages will be enough as spgist searches can determine if there is a match or not only at leaf level.

I have done following things in this week.

1) read the source code of spgist index to understand  the access method

2) found appropriate places to insert calls to existing functions

3) created tests (to verify serialization failures and to demonstrate the feature of reduced false positives) for 'point' and 'box' data types.
    

I will attach the patch shortly.


Regards,
Shubham



Sent with Mailtrack

Attachment

Re: [HACKERS] GSoC 2017: weekly progress reports (week 8)

From
Alexander Korotkov
Date:
Hi!

On Fri, Jul 28, 2017 at 7:58 AM, Shubham Barai <shubhambaraiss@gmail.com> wrote:
I am attaching a patch for predicate locking in SP-Gist index

I took a look over this patch.

newLeafBuffer = SpGistGetBuffer(index,
 GBUF_LEAF | (isNulls ? GBUF_NULLS : 0),
 Min(totalLeafSizes,
 SPGIST_PAGE_CAPACITY),
 &xlrec.initDest);
 PredicateLockPageSplit(index,
 BufferGetBlockNumber(current->buffer),
 BufferGetBlockNumber(newLeafBuffer));

You move predicate lock during split only when new leaf page is allocated.  However SP-GiST may move items to the free space of another busy page during split (see other branches in doPickSplit()).  Your patch doesn't seem to handle this case correctly.

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

Re: [HACKERS] GSoC 2017: weekly progress reports (week 8)

From
Alexander Korotkov
Date:
On Thu, Sep 28, 2017 at 2:22 PM, Alexander Korotkov <a.korotkov@postgrespro.ru> wrote:
On Fri, Jul 28, 2017 at 7:58 AM, Shubham Barai <shubhambaraiss@gmail.com> wrote:
I am attaching a patch for predicate locking in SP-Gist index

I took a look over this patch.

newLeafBuffer = SpGistGetBuffer(index,
 GBUF_LEAF | (isNulls ? GBUF_NULLS : 0),
 Min(totalLeafSizes,
 SPGIST_PAGE_CAPACITY),
 &xlrec.initDest);
 PredicateLockPageSplit(index,
 BufferGetBlockNumber(current->buffer),
 BufferGetBlockNumber(newLeafBuffer));

You move predicate lock during split only when new leaf page is allocated.  However SP-GiST may move items to the free space of another busy page during split (see other branches in doPickSplit()).  Your patch doesn't seem to handle this case correctly.

I've more thoughts about this patch.

+ * SPGist searches acquire predicate lock only on the leaf pages.
+ During a page split, a predicate lock is copied from the original
+ page to the new page.

This approach isn't going to work.  When new tuple is inserted into SP-GiST, choose method can return spgAddNode.  In this case new branch of the tree is added.  And this new branch could probably be used by scans we made in the past.  Therefore, you need to do predicate locking for internal pages too in order to detect all the possible conflicts.

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

Re: [HACKERS] GSoC 2017: weekly progress reports (week 8)

From
Daniel Gustafsson
Date:
> On 29 Sep 2017, at 00:59, Alexander Korotkov <a.korotkov@postgrespro.ru> wrote:
>
> On Thu, Sep 28, 2017 at 2:22 PM, Alexander Korotkov <a.korotkov@postgrespro.ru <mailto:a.korotkov@postgrespro.ru>>
wrote:
> On Fri, Jul 28, 2017 at 7:58 AM, Shubham Barai <shubhambaraiss@gmail.com <mailto:shubhambaraiss@gmail.com>> wrote:
> I am attaching a patch for predicate locking in SP-Gist index
>
> I took a look over this patch.
>
>         newLeafBuffer = SpGistGetBuffer(index,
>                                         GBUF_LEAF | (isNulls ? GBUF_NULLS : 0),
>                                         Min(totalLeafSizes,
>                                             SPGIST_PAGE_CAPACITY),
>                                         &xlrec.initDest);
>         PredicateLockPageSplit(index,
>                     BufferGetBlockNumber(current->buffer),
>                     BufferGetBlockNumber(newLeafBuffer));
>
> You move predicate lock during split only when new leaf page is allocated.  However SP-GiST may move items to the
freespace of another busy page during split (see other branches in doPickSplit()).  Your patch doesn't seem to handle
thiscase correctly. 
>
> I've more thoughts about this patch.
>
> +     * SPGist searches acquire predicate lock only on the leaf pages.
> + During a page split, a predicate lock is copied from the original
> + page to the new page.
>
> This approach isn't going to work.  When new tuple is inserted into SP-GiST, choose method can return spgAddNode.  In
thiscase new branch of the tree is added.  And this new branch could probably be used by scans we made in the past.
Therefore,you need to do predicate locking for internal pages too in order to detect all the possible conflicts. 

Based on this review, I’ve marked this patch Returned with feedback.  Please
re-submit a new version to an upcoming commitfest when ready.

cheers ./daniel

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers