Re: [HACKERS] GSoC 2017: weekly progress reports (week 6)
On 2 October 2017 at 22:21, Alexander Korotkov wrote: > On Sun, Oct 1, 2017 at 11:53 AM, Shubham Barai > wrote: > >> Yes, page-level predicate locking should happen only when fast update is >> off. >> Actually, I forgot to put conditions in updated patch. Does everything >> else look ok ? >> > > I think that isolation tests should be improved. It doesn't seems that > any posting tree would be generated by the tests that you've provided, > because all the TIDs could fit the single posting list. Note, that you can > get some insight into GIN physical structure using pageinspect contrib. > > I have created new isolation tests. Please have a look at updated patch. Regards, Shubham Predicate-locking-in-gin-index_4.patch Description: Binary data
Re: [HACKERS] GSoC 2017 : Patch for predicate locking in Gist index
On 5 January 2018 at 03:18, Alexander Korotkov
wrote:
> On Thu, Jan 4, 2018 at 7:07 PM, Andrey Borodin
> wrote:
>
>> 29 нояб. 2017 г., в 22:50, Shubham Barai
>> написал(а):
>>
>> I have fixed formatting style. Please take a look at updated patch.
>>
>>
>> Here's rebased patch. Every issue has been addressed, so I'm marking this
>> patch as ready for committer.
>>
>
> I'm sorry for concentrating on boring things, but formatting of
> predicate-gist.spec still doesn't look good for me.
>
> # To verify serialization failures, queries and permutations are written
>> in such
>> # a way that an index scan(from one transaction) and an index insert(from
>> another
>> # transaction) will try to access the same part(sub-tree) of the index.
>> #
>> # To check reduced false positives, queries and permutations are written
>> in such
>> # a way that an index scan(from one transaction) and an index insert(from
>> another
>> # transaction) will try to access different parts(sub-tree) of the index.
>>
>
> No space before open bracket (I think it should be when there are multiple
> words brackets).
> Also, we're trying to fit our lines to 80 characters (if it's not
> objectively difficult).
> And these are two almost same paragraphs. I think it should be simplified.
>
> setup
>> {
>> create table gist_point_tbl(id int4, p point);
>> create index gist_pointidx on gist_point_tbl using gist(p);
>> insert into gist_point_tbl (id, p)
>> select g, point(g*10, g*10) from generate_series(1, 1000) g;
>> }
>> setup
>> {
>> BEGIN ISOLATION LEVEL SERIALIZABLE;
>> set enable_seqscan=off;
>> set enable_bitmapscan=off;
>> set enable_indexonlyscan=on;
>> }
>> setup {
>> BEGIN ISOLATION LEVEL SERIALIZABLE;
>> set enable_seqscan=off;
>> set enable_bitmapscan=off;
>> set enable_indexonlyscan=on;
>> }
>
>
> I didn't get idea of using various indentation styles for same purpose.
>
> step "wx3" { insert into gist_point_tbl (id, p)
>> select g, point(g*500, g*500) from generate_series(12,
>> 18) g; }
>
>
> Indented using spaces here...
>
>
I have fixed formatting issues. Please have a look at updated patch.
Regards,
Shubham
Predicate-locking-in-gist-index_7.patch
Description: Binary data
Re: [HACKERS] GSoC 2017 : Patch for predicate locking in Gist index
On 8 January 2018 at 22:44, Shubham Barai wrote:
>
>
> On 5 January 2018 at 03:18, Alexander Korotkov
> wrote:
>
>> On Thu, Jan 4, 2018 at 7:07 PM, Andrey Borodin
>> wrote:
>>
>>> 29 нояб. 2017 г., в 22:50, Shubham Barai
>>> написал(а):
>>>
>>> I have fixed formatting style. Please take a look at updated patch.
>>>
>>>
>>> Here's rebased patch. Every issue has been addressed, so I'm marking
>>> this patch as ready for committer.
>>>
>>
>> I'm sorry for concentrating on boring things, but formatting of
>> predicate-gist.spec still doesn't look good for me.
>>
>> # To verify serialization failures, queries and permutations are written
>>> in such
>>> # a way that an index scan(from one transaction) and an index
>>> insert(from another
>>> # transaction) will try to access the same part(sub-tree) of the index.
>>> #
>>> # To check reduced false positives, queries and permutations are written
>>> in such
>>> # a way that an index scan(from one transaction) and an index
>>> insert(from another
>>> # transaction) will try to access different parts(sub-tree) of the index.
>>>
>>
>> No space before open bracket (I think it should be when there are
>> multiple words brackets).
>> Also, we're trying to fit our lines to 80 characters (if it's not
>> objectively difficult).
>> And these are two almost same paragraphs. I think it should be
>> simplified.
>>
>> setup
>>> {
>>> create table gist_point_tbl(id int4, p point);
>>> create index gist_pointidx on gist_point_tbl using gist(p);
>>> insert into gist_point_tbl (id, p)
>>> select g, point(g*10, g*10) from generate_series(1, 1000) g;
>>> }
>>> setup
>>> {
>>> BEGIN ISOLATION LEVEL SERIALIZABLE;
>>> set enable_seqscan=off;
>>> set enable_bitmapscan=off;
>>> set enable_indexonlyscan=on;
>>> }
>>> setup {
>>> BEGIN ISOLATION LEVEL SERIALIZABLE;
>>> set enable_seqscan=off;
>>> set enable_bitmapscan=off;
>>> set enable_indexonlyscan=on;
>>> }
>>
>>
>> I didn't get idea of using various indentation styles for same purpose.
>>
>> step "wx3" { insert into gist_point_tbl (id, p)
>>> select g, point(g*500, g*500) from generate_series(12,
>>> 18) g; }
>>
>>
>> Indented using spaces here...
>>
>>
>
>
I have fixed formatting issues. Please have a look at updated patch.
Regards,
Shubham
Predicate-locking-in-gist_index_v7.patch
Description: Binary data
Re: [HACKERS] GSoC 2017 : Patch for predicate locking in Gist index
On 8 January 2018 at 23:13, Shubham Barai wrote:
>
>
> On 8 January 2018 at 22:44, Shubham Barai
> wrote:
>
>>
>>
>> On 5 January 2018 at 03:18, Alexander Korotkov > > wrote:
>>
>>> On Thu, Jan 4, 2018 at 7:07 PM, Andrey Borodin
>>> wrote:
>>>
>>>> 29 нояб. 2017 г., в 22:50, Shubham Barai
>>>> написал(а):
>>>>
>>>> I have fixed formatting style. Please take a look at updated patch.
>>>>
>>>>
>>>> Here's rebased patch. Every issue has been addressed, so I'm marking
>>>> this patch as ready for committer.
>>>>
>>>
>>> I'm sorry for concentrating on boring things, but formatting of
>>> predicate-gist.spec still doesn't look good for me.
>>>
>>> # To verify serialization failures, queries and permutations are written
>>>> in such
>>>> # a way that an index scan(from one transaction) and an index
>>>> insert(from another
>>>> # transaction) will try to access the same part(sub-tree) of the index.
>>>> #
>>>> # To check reduced false positives, queries and permutations are
>>>> written in such
>>>> # a way that an index scan(from one transaction) and an index
>>>> insert(from another
>>>> # transaction) will try to access different parts(sub-tree) of the
>>>> index.
>>>>
>>>
>>> No space before open bracket (I think it should be when there are
>>> multiple words brackets).
>>> Also, we're trying to fit our lines to 80 characters (if it's not
>>> objectively difficult).
>>> And these are two almost same paragraphs. I think it should be
>>> simplified.
>>>
>>> setup
>>>> {
>>>> create table gist_point_tbl(id int4, p point);
>>>> create index gist_pointidx on gist_point_tbl using gist(p);
>>>> insert into gist_point_tbl (id, p)
>>>> select g, point(g*10, g*10) from generate_series(1, 1000) g;
>>>> }
>>>> setup
>>>> {
>>>> BEGIN ISOLATION LEVEL SERIALIZABLE;
>>>> set enable_seqscan=off;
>>>> set enable_bitmapscan=off;
>>>> set enable_indexonlyscan=on;
>>>> }
>>>> setup {
>>>> BEGIN ISOLATION LEVEL SERIALIZABLE;
>>>> set enable_seqscan=off;
>>>> set enable_bitmapscan=off;
>>>> set enable_indexonlyscan=on;
>>>> }
>>>
>>>
>>> I didn't get idea of using various indentation styles for same purpose.
>>>
>>> step "wx3" { insert into gist_point_tbl (id, p)
>>>> select g, point(g*500, g*500) from
>>>> generate_series(12, 18) g; }
>>>
>>>
>>> Indented using spaces here...
>>>
>>>
>>
>>
> I have fixed formatting issues. Please have a look at updated patch.
>
>
The previous patch couldn't be applied cleanly because there were some
modifications to isolation_schedule. I have updated the patch now.
Regards,
Shubham
Predicate-Locking-in-gist-index_v8.patch
Description: Binary data
Re: [HACKERS] GSoC 2017: weekly progress reports (week 4) and patch for hash index
On 15 January 2018 at 08:03, Stephen Frost wrote: > Greeting Shubham, all, > > * Michael Paquier (michael.paqu...@gmail.com) wrote: > > On Mon, Sep 25, 2017 at 10:34 PM, Shubham Barai > > wrote: > > > I have attached the rebased version of patch here. > > > > The patch does not apply and there has been no reviews as well. In > > consequence, I am moving it to next CF with "waiting on author" as > > status. Please provide a rebased patch. > > Shubham, would you mind providing an updated patch which applies > cleanly, so we can change this to Needs Review and hopefully get someone > looking at it? Also, it would be good to respond to Alexander's as to > if it would work or not (and perhaps updating the patch accordingly). > Otherwise, I'm afriad this patch may end up just getting bumped to the > next CF or ending up as 'returned with feedback'. Would be great to get > this up to a point where it could be committed. > > > Hi Stephen, The new approach was suggested after completion of GSoC. So, I didn't get enough time to implement this approach. Also, I was constantly updating my patches for gist and gin index based on reviews. Here, I am attaching the rebased version of the patch (which is based on an old approch: acquiring a predicate lock on primary page of a bucket) Kind Regards, Shubham Predicate-Locking-in-hash-index_v5.patch Description: Binary data
Re: [HACKERS] GSoC 2017 : Patch for predicate locking in Gist index
On 25 January 2018 at 18:40, Alexander Korotkov wrote: > On Wed, Jan 10, 2018 at 9:55 PM, Shubham Barai > wrote: > >> The previous patch couldn't be applied cleanly because there were some >> modifications to isolation_schedule. I have updated the patch now. >> > > In the attached patch indentation is still looking strange. > I've contacted Shubham using Telegram, and we realized that > it's because he used tab width 8 in his editor. > Shumham seems to have updated version of this patch, but didn't > post it yet. Thus, I'm marking this "Waiting on author" until > the updated patch is posted. > > > I have fixed formatting issues. Please take a look at updated patch. Regards, Shubham Predicate-Locking-in-gist-index_v9.patch Description: Binary data
Re: [HACKERS] GSoC 2017: weekly progress reports (week 6)
On 28 February 2018 at 05:51, Thomas Munro wrote: > On Wed, Jan 3, 2018 at 4:31 AM, Shubham Barai > wrote: > > I have created new isolation tests. Please have a look at > > updated patch. > > Hi Shubham, > > Could we please have a rebased version of the gin one? > Sure. I have attached a rebased version Regards, Shubham Predicate-Locking-in-gin-index_v5.patch Description: Binary data
Re: [HACKERS] GSoC 2017: weekly progress reports (week 6)
On 07-Mar-2018 11:00 PM, "Alvaro Herrera" wrote: I suggest to create a new function GinPredicateLockPage() that checks whether fast update is enabled for the index. The current arrangement looks too repetitive and it seems easy to make a mistake. Stylistically, please keep #include lines ordered alphabetically, and cut long lines to below 80 chars. Okay, I will update the patch.
Re: [HACKERS] GSoC 2017: weekly progress reports (week 6)
On 16 March 2018 at 03:57, Alexander Korotkov wrote: > On Tue, Mar 13, 2018 at 3:25 PM, Alvaro Herrera > wrote: > >> Alexander Korotkov wrote: >> >> > And what happen if somebody concurrently set (fastupdate = on)? >> > Can we miss conflicts because of that? >> >> I think it'd be better to have that option require AccessExclusive lock, >> so that it can never be changed concurrently with readers. Seems to me >> that penalizing every single read to cope with this case would be a bad >> trade-off. > > > As Andrey Borodin mentioned, we already do. Sorry for buzz :) > > > I have updated the patch based on suggestions. Regards, Shubham Predicate-Locking-in-gin-index_v6.patch Description: Binary data
Re: [HACKERS] GSoC 2017 : Patch for predicate locking in Gist index
On 27 November 2017 at 13:17, Alexander Korotkov wrote: > Hi, Shubham! > > On Wed, Nov 1, 2017 at 12:10 AM, Shubham Barai > wrote: > >> On 9 October 2017 at 18:57, Alexander Korotkov > > wrote: >> >>> Now, ITSM that predicate locks and conflict checks are placed right for >>> now. >>> However, it would be good to add couple comments to gistdoinsert() whose >>> would state why do we call CheckForSerializableConflictIn() in these >>> particular places. >>> >>> I also take a look at isolation tests. You made two separate test >>> specs: one to verify that serialization failures do fire, and another to >>> check there are no false positives. >>> I wonder if we could merge this two test specs into one, but use more >>> variety of statements with different keys for both inserts and selects. >>> >> >> Please find the updated version of patch here. I have made suggested >> changes. >> > > In general, patch looks good for me now. I just see some cosmetic issues. > > /* >> + *Check for any r-w conflicts (in serialisation isolation level) >> + *just before we intend to modify the page >> + */ >> + CheckForSerializableConflictIn(r, NULL, stack->buffer); >> + /* > > > Formatting doesn't look good here. You've missed space after star sign in > the comment. You also missed newline after CheckForSerializableConflictIn() > call. > > Also, you've long comment lines in predicate-gist.spec. Please, break > long comments into multiple lines. > > I have fixed formatting style. Please take a look at updated patch. Regards, Shubham > > From d7780debdcce60340aebcef06bb03f12419dbbeb Mon Sep 17 00:00:00 2001 From: shubhambaraiss Date: Sun, 1 Oct 2017 23:42:41 +0530 Subject: [PATCH] Predicate locking in gist index --- src/backend/access/gist/gist.c | 21 +- src/backend/access/gist/gistget.c | 3 + src/backend/storage/lmgr/README-SSI| 5 +- src/test/isolation/expected/predicate-gist.out | 659 + src/test/isolation/isolation_schedule | 2 + src/test/isolation/specs/predicate-gist.spec | 117 + 6 files changed, 804 insertions(+), 3 deletions(-) mode change 100644 => 100755 src/backend/access/gist/gist.c mode change 100644 => 100755 src/backend/access/gist/gistget.c mode change 100644 => 100755 src/backend/storage/lmgr/README-SSI create mode 100644 src/test/isolation/expected/predicate-gist.out create mode 100644 src/test/isolation/specs/predicate-gist.spec diff --git a/src/backend/access/gist/gist.c b/src/backend/access/gist/gist.c old mode 100644 new mode 100755 index 565525b..74e8c7c --- a/src/backend/access/gist/gist.c +++ b/src/backend/access/gist/gist.c @@ -18,6 +18,8 @@ #include "access/gistscan.h" #include "catalog/pg_collation.h" #include "miscadmin.h" +#include "storage/lmgr.h" +#include "storage/predicate.h" #include "nodes/execnodes.h" #include "utils/builtins.h" #include "utils/index_selfuncs.h" @@ -70,7 +72,7 @@ gisthandler(PG_FUNCTION_ARGS) amroutine->amsearchnulls = true; amroutine->amstorage = true; amroutine->amclusterable = true; - amroutine->ampredlocks = false; + amroutine->ampredlocks = true; amroutine->amcanparallel = false; amroutine->amkeytype = InvalidOid; @@ -446,6 +448,11 @@ gistplacetopage(Relation rel, Size freespace, GISTSTATE *giststate, GistPageSetNSN(ptr->page, oldnsn); } + for (ptr = dist; ptr; ptr = ptr->next) + PredicateLockPageSplit(rel, + BufferGetBlockNumber(buffer), + BufferGetBlockNumber(ptr->buffer)); + /* * gistXLogSplit() needs to WAL log a lot of pages, prepare WAL * insertion for that. NB: The number of pages and data segments @@ -734,6 +741,12 @@ gistdoinsert(Relation r, IndexTuple itup, Size freespace, GISTSTATE *giststate) } /* + * Check for any r-w conflicts (in serialisation isolation level) + * just before we intend to modify the page + */ +CheckForSerializableConflictIn(r, NULL, stack->buffer); + +/* * Update the tuple. * * We still hold the lock after gistinserttuple(), but it @@ -827,6 +840,12 @@ gistdoinsert(Relation r, IndexTuple itup, Size freespace, GISTSTATE *giststate) } } + /* + * Check for any r-w conflicts (in serialisation isolation level) + * just before we intend to modify the page + */ + CheckForSerializableConflictIn(r, NULL, stack->buffer); + /* now state.stack->(page, buffer and blkno) points to leaf page */ gistinserttuple(&state, stack, giststate, itup, diff --git a/src/backend/access/gist/gistget.c b/src/backend/access/gist/gistget.
