|From:||Shubham Barai <shubhambaraiss(at)gmail(dot)com>|
|To:||Alexander Korotkov <a(dot)korotkov(at)postgrespro(dot)ru>|
|Cc:||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 6)|
|Views:||Raw Message | Whole Thread | Download mbox|
On 28 September 2017 at 15:49, Alexander Korotkov <a(dot)korotkov(at)postgrespro(dot)ru
> On Thu, Sep 28, 2017 at 12:45 AM, Alexander Korotkov <
> a(dot)korotkov(at)postgrespro(dot)ru> wrote:
>> On Wed, Aug 9, 2017 at 6:30 PM, Shubham Barai <shubhambaraiss(at)gmail(dot)com>
>>> Please find the updated patch for predicate locking in gin index here.
>>> There was a small issue in the previous patch. I didn't consider the case
>>> where only root page exists in the tree, and there is a predicate lock
>>> on it,
>>> and it gets split.
>>> If we treat the original page as a left page and create a new root and
>>> page, then we just need to copy a predicate lock from the left to the
>>> page (this is the case in B-tree).
>>> But if we treat the original page as a root and create a new left and
>>> page, then we need to copy a predicate lock on both new pages (in the
>>> case of rum and gin).
>>> link to updated code and tests: https://github.com/shub
>> I've assigned to review this patch. First of all I'd like to understand
>> general idea of this patch.
>> As I get, you're placing predicate locks to both entry tree leaf pages
>> and posting tree leaf pages. But, GIN implements so called "fast scan"
>> technique which allows it to skip some leaf pages of posting tree when
>> these pages are guaranteed to not contain matching item pointers. Wherein
>> the particular order of posting list scan and skip depends of their
>> estimated size (so it's a kind of coincidence).
>> But thinking about this more generally, I found that proposed locking
>> scheme is redundant. Currently when entry has posting tree, you're locking
>> 1) entry tree leaf page containing pointer to posting tree,
>> 2) leaf pages of corresponding posting tree.
>> Therefore conflicting transactions accessing same entry would anyway
>> conflict while accessing the same entry tree leaf page. So, there is no
>> necessity to lock posting tree leaf pages at all. Alternatively, if entry
>> has posting tree, you can skip locking entry tree leaf page and lock
>> posting tree leaf pages instead.
> I'd like to note that I had following warnings during compilation using
> gininsert.c:519:47: warning: incompatible pointer to integer conversion
>> passing 'void *' to parameter of type 'Buffer' (aka 'int')
>> CheckForSerializableConflictIn(index, NULL, NULL);
>> note: expanded from macro 'NULL'
>> # define NULL ((void*)0)
>> ../../../../src/include/storage/predicate.h:64:87: note: passing
>> argument to parameter 'buffer' here
>> extern void CheckForSerializableConflictIn(Relation relation, HeapTuple
>> tuple, Buffer buffer);
>> 1 warning generated.
>> ginvacuum.c:163:2: warning: implicit declaration of function
>> 'PredicateLockPageCombine' is invalid in C99 [-Wimplicit-function-
>> PredicateLockPageCombine(gvs->index, deleteBlkno, rightlink);
>> 1 warning generated.
> Also, I tried to remove predicate locks from posting tree leafs. At least
> isolation tests passed correctly after this change.
> However, after telegram discussion with Andrew Borodin, we decided that it
> would be better to do predicate locking and conflict checking for posting
> tree leafs, but skip that for entry tree leafs (in the case when entry has
> posting tree). That would give us more granular locking and less false
I have made changes according to your suggestions. Please have a look at
the updated patch.
I am also considering your suggestions for my other patches also. But, I
will need some time to
make changes as I am currently busy doing my master's.
|Next Message||Alexander Korotkov||2017-09-30 20:17:22||Re: GSoC 2017: weekly progress reports (week 6)|
|Previous Message||Michael Paquier||2017-09-29 22:51:01||Re: [HACKERS] User-perspective knowledge about wait events|
|Next Message||Tom Lane||2017-09-30 15:15:01||Re: extension build issue with PostgreSQL 10 on Centos6|
|Previous Message||Robert Haas||2017-09-30 14:34:25||Re: 64-bit queryId?|