Thanks to everyone, v3 is pushed.
Teodor Sigaev wrote:
I don't very happy with rootBuffer added everywhere. ginInsertItemPointers()
and ginPrepareDataScan() now take both args, rootBlkno and rootBuffer,
second could be invalid. As I can see, you did it to call
CheckForSerializableConflictIn(
I don't very happy with rootBuffer added everywhere. ginInsertItemPointers() and
ginPrepareDataScan() now take both args, rootBlkno and rootBuffer, second
could be invalid. As I can see, you did it to call
CheckForSerializableConflictIn() in ginEntryInsert(). Seems, it could be
reverted and C
I took a stab at fixing those issues, as well as the bug when fastupdate is
turned on concurrently. Does the attached patch look sane to you?
That's look sane, and I believe it should be applied but I see some issue in
your patch:
I don't very happy with rootBuffer added everywhere. ginInsertIt
> 9 апр. 2018 г., в 23:04, Heikki Linnakangas написал(а):
>
> On 09/04/18 18:21, Andrey Borodin wrote:
>>> 9 апр. 2018 г., в 19:50, Teodor Sigaev
>>> написал(а):
3. Why do we *not* lock the entry leaf page, if there is no
match? We still need a lock to remember that we probed for tha
On 09/04/18 18:21, Andrey Borodin wrote:
9 апр. 2018 г., в 19:50, Teodor Sigaev
написал(а):
3. Why do we *not* lock the entry leaf page, if there is no
match? We still need a lock to remember that we probed for that
value and there was no match, so that we conflict with a tuple
that might be
On 09/04/18 18:04, Alvaro Herrera wrote:
Heikki Linnakangas wrote:
Remember, the purpose of predicate locks is to lock key ranges, not physical
pages or tuples in the index. We use leaf pages as handy shortcut for "any
key value that would belong on this page", but it is just an implementation
Alvaro Herrera wrote:
Heikki Linnakangas wrote:
Remember, the purpose of predicate locks is to lock key ranges, not physical
pages or tuples in the index. We use leaf pages as handy shortcut for "any
key value that would belong on this page", but it is just an implementation
detail.
Hmm ...
> 9 апр. 2018 г., в 19:50, Teodor Sigaev написал(а):
>>
>> 3. Why do we *not* lock the entry leaf page, if there is no match? We still
>> need a lock to remember that we probed for that value and there was no
>> match, so that we conflict with a tuple that might be inserted later.
>> At least
Heikki Linnakangas wrote:
> Remember, the purpose of predicate locks is to lock key ranges, not physical
> pages or tuples in the index. We use leaf pages as handy shortcut for "any
> key value that would belong on this page", but it is just an implementation
> detail.
Hmm ... so, thinking about
Hi!
Thank you for taking a look at this patch. I really appreciate your
attention over complex subjects like this.
On Mon, Apr 9, 2018 at 1:33 PM, Heikki Linnakangas wrote:
> On 28/03/18 19:53, Teodor Sigaev wrote:
>
>> As I understand, scan should lock any visited page, but now it's true
>> o
Hi!
1. Why do we lock all posting tree pages, even though they all represent the
same value? Isn't it enough to lock the root of the posting tree?
2. Why do we lock any posting tree pages at all, if we lock the entry tree page
anyway? Isn't the lock on the entry tree page sufficient to cover t
Ugh, I miss your last email where you another locking protocol. Reading.
Teodor Sigaev wrote:
Attached is a test case that demonstrates a case where we miss a serialization
failure, when fastupdate is turned on concurrently. It works on v10, but fails
to throw a serialization error on v11.
Tha
Attached is a test case that demonstrates a case where we miss a serialization
failure, when fastupdate is turned on concurrently. It works on v10, but fails
to throw a serialization error on v11.
Thank you for reserching!
Proof of concept:
diff --git a/src/backend/commands/tablecmds.c b/src/ba
On 28/03/18 19:53, Teodor Sigaev wrote:
Hi!
I slightly modified test for clean demonstration of difference between
fastupdate on and off. Also I added CheckForSerializableConflictIn() to
fastupdate off codepath, but only in case of non-empty pending list.
The next question what I see: why do no
On 16/03/18 00:26, Alexander Korotkov wrote:
On Tue, Mar 13, 2018 at 3:26 PM, Andrey Borodin wrote:
On 13/03/18 14:02, Alexander Korotkov wrote:
And what happen if somebody concurrently set (fastupdate = on)?
Can we miss conflicts because of that?
No, AccessExclusiveLock will prevent this ki
I wrote:
> prion doesn't like this patch, which probably means that something is
> trying to use the content of a relcache entry after having closed it.
Oh, sorry, scratch that --- looking closer, it was failing before this.
regards, tom lane
Teodor Sigaev writes:
> Thanks for everyone, pushed
prion doesn't like this patch, which probably means that something is
trying to use the content of a relcache entry after having closed it.
regards, tom lane
Thanks for everyone, pushed
Oh, right, that makes sense. I'm not sure that the comments explain
this sufficiently -- I think it'd be good to expand that.
I improved comments
Given this patch, it seems clear that serializable mode is much worse
with fastupdate than without it!
That's true. An
Teodor Sigaev wrote:
>
> Alvaro Herrera wrote:
> > I don't quite understand the new call in gininsert -- I mean I see that
> > it wants to check for conflicts even when fastupdate is set, but why?
> If fastupdate is set then we check conflict with whole index, not a
> particular pages in it. Predi
Alvaro Herrera wrote:
I don't quite understand the new call in gininsert -- I mean I see that
it wants to check for conflicts even when fastupdate is set, but why?
If fastupdate is set then we check conflict with whole index, not a particular
pages in it. Predicate lock on penging list pages w
On Thu, Mar 29, 2018 at 1:38 PM, Teodor Sigaev wrote:
> The next question what I see: why do not we lock entry leaf pages in some
>> cases?
>>
>
> I've modified patch to predicate lock each leaf (entry or posting) page.
> Now patch reaches commitable state from my point view.
I made some enhanc
I don't quite understand the new call in gininsert -- I mean I see that
it wants to check for conflicts even when fastupdate is set, but why?
Maybe, just maybe, it would be better to add a new flag to the
GinCheckForSerializableConflictIn function, that's passed differently
for this one callsite, a
The next question what I see: why do not we lock entry leaf pages in some cases?
I've modified patch to predicate lock each leaf (entry or posting) page. Now
patch reaches commitable state from my point view.
--
Teodor Sigaev E-mail: teo...@sigaev.ru
Hi!
I slightly modified test for clean demonstration of difference between
fastupdate on and off. Also I added CheckForSerializableConflictIn() to
fastupdate off codepath, but only in case of non-empty pending list.
The next question what I see: why do not we lock entry leaf pages in some cas
I'd like to see fastupdate=on in test too, now tests cover only case
without fastupdate. Please, add them.
Here's a couple of tests for pending list (fastupdate = on).
By the way, isn't it strange that permutation "fu1" "rxy3" "wx3" "rxy4"
"c1" "wy4" "c2" doesn't produce an ERROR?
--
Dmitry
I'd like to see fastupdate=on in test too, now tests cover only case
without fastupdate. Please, add them.
Here's a couple of tests for pending list (fastupdate = on).
--
Dmitry Ivanov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Companydiff --git a/src/backend/access/
Hi!
Patch seems good, but I found one bug in it, in fact, nobody
checks serializible conflict with fastupdate=on:
gininsert()
{
if (GinGetUseFastUpdate())
{
/* two next lines are GinCheckForSerializableConflictIn() */
if (!GinGetUseFastUpdate())
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
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 b
On Tue, Mar 13, 2018 at 3:26 PM, Andrey Borodin
wrote:
> > 13 марта 2018 г., в 17:02, Alexander Korotkov
> написал(а):
> >
> > BTW to BTW. I think we should check pending list size with
> GinGetPendingListCleanupSize() here
> > +
> > + /*
> > +* If fast update is enabled, we acquir
> 13 марта 2018 г., в 17:02, Alexander Korotkov
> написал(а):
>
> BTW to BTW. I think we should check pending list size with
> GinGetPendingListCleanupSize() here
> +
> + /*
> +* If fast update is enabled, we acquire a predicate lock on the
> entire
> +* relation as fas
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 eve
On Mon, Mar 12, 2018 at 9:47 AM, Andrey Borodin
wrote:
> > 12 марта 2018 г., в 1:54, Alexander Korotkov
> написал(а):
> >
> > On Wed, Mar 7, 2018 at 8:30 PM, Alvaro Herrera
> wrote:
> > I suggest to create a new function GinPredicateLockPage() that checks
> > whether fast update is enabled for
> 12 марта 2018 г., в 1:54, Alexander Korotkov
> написал(а):
>
> On Wed, Mar 7, 2018 at 8:30 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
On Wed, Mar 7, 2018 at 8:30 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.
>
BTW, should we also skip CheckForSe
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 ordere
Hi,
On 2018-03-07 11:58:51 -0300, Alvaro Herrera wrote:
> > This appears to be a duplicate of
> > https://commitfest.postgresql.org/17/1466/ - as the other one is older, I'm
> > closing this one.
>
> This comment makes no sense from the POV of the mail archives. I had to
> look at the User-Age
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 ch
Andres Freund wrote:
> This appears to be a duplicate of https://commitfest.postgresql.org/17/1466/
> - as the other one is older, I'm closing this one.
This comment makes no sense from the POV of the mail archives. I had to
look at the User-Agent in your email to realize that you wrote it in t
Hi!
> 28 февр. 2018 г., в 22:19, Shubham Barai
> написал(а):
>
> Sure. I have attached a rebased version
I've looked into the code closely again. The patch is heavily reworked since
GSoC state :)
Tests are looking fine and locking is fine-grained.
But there is one thing I could not understand
This appears to be a duplicate of https://commitfest.postgresql.org/17/1466/ -
as the other one is older, I'm closing this one.
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 rebase
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?
--
Thomas Munro
http://www.enterprisedb.com
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 ?
>>
>
>
On Tue, Oct 3, 2017 at 1:51 AM, Alexander Korotkov
wrote:
> 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
> insi
45 matches
Mail list logo
