Re: GSoC 2017: weekly progress reports (week 8)

From: Daniel Gustafsson <daniel(at)yesql(dot)se>
To: Shubham Barai <shubhambaraiss(at)gmail(dot)com>
Cc: Alexander Korotkov <a(dot)korotkov(at)postgrespro(dot)ru>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, Andrew Borodin <amborodin86(at)gmail(dot)com>, Kevin Grittner <kgrittn(at)gmail(dot)com>
Subject: Re: GSoC 2017: weekly progress reports (week 8)
Date: 2017-10-02 10:18:51
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

> On 29 Sep 2017, at 00:59, Alexander Korotkov <a(dot)korotkov(at)postgrespro(dot)ru> wrote:
> On Thu, Sep 28, 2017 at 2:22 PM, Alexander Korotkov <a(dot)korotkov(at)postgrespro(dot)ru <mailto:a(dot)korotkov(at)postgrespro(dot)ru>> wrote:
> On Fri, Jul 28, 2017 at 7:58 AM, Shubham Barai <shubhambaraiss(at)gmail(dot)com <mailto:shubhambaraiss(at)gmail(dot)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,
> &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.

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

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Kyotaro HORIGUCHI 2017-10-02 10:22:03 Re: proposal - Default namespaces for XPath expressions (PostgreSQL 11)
Previous Message Daniel Gustafsson 2017-10-02 10:13:18 Re: Transactions involving multiple postgres foreign servers