GSoC 2017: weekly progress reports (week 8)

Started by Shubham Baraiabout 9 years ago5 messageshackers
Beta feature

Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.

won't retrysuccessCI history

You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:

docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t37164
psql -h localhost -U postgres

Built from patchset v2 (message #2), July 28, 2026 at 05:47 AM.

Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:

git clone --branch t37164_2 https://github.com/hackorum-dev/postgres.git

In a checkout you already have, add the fork once:

git remote add hackorum https://github.com/hackorum-dev/postgres.git

then, for this patchset and every later one:

git fetch hackorum t37164_2 && git checkout t37164_2

Patchset v2 (message #2) is on t37164_2

Jump to latest
#1Shubham Barai
shubhambaraiss@gmail.com

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.

link to code and tests: https://github.com/shub
hambaraiss/postgres/commit/d9ae709c93ff038478ada411c621faeabe6813cb

I will attach the patch shortly.

Regards,
Shubham

<https://mailtrack.io/&gt; Sent with Mailtrack
<https://mailtrack.io/install?source=signature&amp;lang=en&amp;referral=shubhambaraiss@gmail.com&amp;idSignature=22&gt;

#2Shubham Barai
shubhambaraiss@gmail.com
In reply to: Shubham Barai (#1)
Re: GSoC 2017: weekly progress reports (week 8)

Hi.

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

Regards,
Shubham

<https://mailtrack.io/&gt; Sent with Mailtrack
<https://mailtrack.io/install?source=signature&amp;lang=en&amp;referral=shubhambaraiss@gmail.com&amp;idSignature=22&gt;

On 26 July 2017 at 20:50, Shubham Barai <shubhambaraiss@gmail.com> wrote:

Show quoted text

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.

link to code and tests: https://github.com/shub
hambaraiss/postgres/commit/d9ae709c93ff038478ada411c621faeabe6813cb

I will attach the patch shortly.

Regards,
Shubham

<https://mailtrack.io/&gt; Sent with Mailtrack
<https://mailtrack.io/install?source=signature&amp;lang=en&amp;referral=shubhambaraiss@gmail.com&amp;idSignature=22&gt;

Attachments:

t37164_2
Predicate-Locking-in-spgist-index.patchapplication/octet-stream; name=Predicate-Locking-in-spgist-index.patchDownload+933-2
#3Alexander Korotkov
aekorotkov@gmail.com
In reply to: Shubham Barai (#2)
Re: GSoC 2017: weekly progress reports (week 8)

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

#4Alexander Korotkov
aekorotkov@gmail.com
In reply to: Alexander Korotkov (#3)
Re: GSoC 2017: weekly progress reports (week 8)

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

#5Daniel Gustafsson
daniel@yesql.se
In reply to: Alexander Korotkov (#4)
Re: GSoC 2017: weekly progress reports (week 8)

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 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

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