Обсуждение: GIN predicate locking slows down valgrind isolationtests tremendously

Hi,

https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=skink&dt=2018-03-31%2014%3A40%3A02
isolation-check (02:19:36)

https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=skink&dt=2018-03-28%2010%3A40%3A02
isolation-check (00:18:44)

As far as I can tell that increase comes laregely from the new GIN
tests.  Could one of you please look at keeping the test time increase
to something more reasonable?

Greetings,

Andres Freund

On 2018-12-03 16:07:40 -0800, Andres Freund wrote:
> Hi,
> 
> https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=skink&dt=2018-03-31%2014%3A40%3A02
> isolation-check (02:19:36)
> 
> https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=skink&dt=2018-03-28%2010%3A40%3A02
> isolation-check (00:18:44)
> 
> As far as I can tell that increase comes laregely from the new GIN
> tests.  Could one of you please look at keeping the test time increase
> to something more reasonable?

Ping?

It's also one of the slowest tests outside of valgrind...

Greetings,

Andres Freund

On Fri, Dec 21, 2018 at 1:50 AM Alexander Korotkov
<a.korotkov@postgrespro.ru> wrote:
> чт, 20 дек. 2018 г., 2:22 Andres Freund andres@anarazel.de:
>> On 2018-12-03 16:07:40 -0800, Andres Freund wrote:
>> > As far as I can tell that increase comes laregely from the new GIN
>> > tests.  Could one of you please look at keeping the test time increase
>> > to something more reasonable?
>>
>> Ping?
>>
>> It's also one of the slowest tests outside of valgrind...
>
> I'm going to take a look on that.

While trying to reduce isolation test suite for GIN, I found following
behavior surprising for me.

GinBtreeStack *
ginFindLeafPage(GinBtree btree, bool searchMode, Snapshot snapshot)
{
    GinBtreeStack *stack;

    stack = (GinBtreeStack *) palloc(sizeof(GinBtreeStack));
    stack->blkno = btree->rootBlkno;
    stack->buffer = ReadBuffer(btree->index, btree->rootBlkno);
    stack->parent = NULL;
    stack->predictNumber = 1;

    if (!searchMode)
        CheckForSerializableConflictIn(btree->index, NULL, stack->buffer);

So, we're checking for conflict on tree root for every entry insert.
That's right for posting tree, but completely unneeded for entry tree.
I'm intended to change that to lock root of only posting tree if
nobody explains me why I'm wrong...

------
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

On Fri, Dec 21, 2018 at 1:50 AM Alexander Korotkov
<a.korotkov@postgrespro.ru> wrote:
> чт, 20 дек. 2018 г., 2:22 Andres Freund andres@anarazel.de:
>> On 2018-12-03 16:07:40 -0800, Andres Freund wrote:
>> > As far as I can tell that increase comes laregely from the new GIN
>> > tests.  Could one of you please look at keeping the test time increase
>> > to something more reasonable?
>>
>> Ping?
>>
>> It's also one of the slowest tests outside of valgrind...
>
> I'm going to take a look on that.

BTW, patch for reducing isolation testing for gin predicate locking is
attached.  Could anybody check its execution time with valgrind (and
compare with execution time of previous test suite)?

------
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

On 12/24/18 5:25 PM, Alexander Korotkov wrote:
> On Fri, Dec 21, 2018 at 1:50 AM Alexander Korotkov
> <a.korotkov@postgrespro.ru> wrote:
>> чт, 20 дек. 2018 г., 2:22 Andres Freund andres@anarazel.de:
>>> On 2018-12-03 16:07:40 -0800, Andres Freund wrote:
>>>> As far as I can tell that increase comes laregely from the new GIN
>>>> tests.  Could one of you please look at keeping the test time increase
>>>> to something more reasonable?
>>> Ping?
>>>
>>> It's also one of the slowest tests outside of valgrind...
>> I'm going to take a look on that.
> BTW, patch for reducing isolation testing for gin predicate locking is
> attached.  Could anybody check its execution time with valgrind (and
> compare with execution time of previous test suite)?



Will try if I get some time to spare in the next 72 hours or so


cheers


andrew


-- 
Andrew Dunstan                https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On Tue, Dec 25, 2018 at 1:48 AM Andrew Dunstan
<andrew.dunstan@2ndquadrant.com> wrote:
> On 12/24/18 5:25 PM, Alexander Korotkov wrote:
> > On Fri, Dec 21, 2018 at 1:50 AM Alexander Korotkov
> > <a.korotkov@postgrespro.ru> wrote:
> >> чт, 20 дек. 2018 г., 2:22 Andres Freund andres@anarazel.de:
> >>> On 2018-12-03 16:07:40 -0800, Andres Freund wrote:
> >>>> As far as I can tell that increase comes laregely from the new GIN
> >>>> tests.  Could one of you please look at keeping the test time increase
> >>>> to something more reasonable?
> >>> Ping?
> >>>
> >>> It's also one of the slowest tests outside of valgrind...
> >> I'm going to take a look on that.
> > BTW, patch for reducing isolation testing for gin predicate locking is
> > attached.  Could anybody check its execution time with valgrind (and
> > compare with execution time of previous test suite)?
>
> Will try if I get some time to spare in the next 72 hours or so

Thank you!

------
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

On Tue, Dec 25, 2018 at 12:19 AM Alexander Korotkov
<a.korotkov@postgrespro.ru> wrote:
> So, we're checking for conflict on tree root for every entry insert.
> That's right for posting tree, but completely unneeded for entry tree.
> I'm intended to change that to lock root of only posting tree if
> nobody explains me why I'm wrong...

BTW, I've tried to remove conflict checking from entry tree root
(patch is attached).  Neither current or my version of isolation tests
appear to be affected.

------
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

On Tue, Dec 25, 2018 at 4:32 AM Alexander Korotkov
<a.korotkov@postgrespro.ru> wrote:
> On Tue, Dec 25, 2018 at 12:19 AM Alexander Korotkov
> <a.korotkov@postgrespro.ru> wrote:
> > So, we're checking for conflict on tree root for every entry insert.
> > That's right for posting tree, but completely unneeded for entry tree.
> > I'm intended to change that to lock root of only posting tree if
> > nobody explains me why I'm wrong...
>
> BTW, I've tried to remove conflict checking from entry tree root
> (patch is attached).  Neither current or my version of isolation tests
> appear to be affected.

Given no comments yet, I'll commit this on no objection.

------
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

On Tue, Dec 25, 2018 at 1:25 AM Alexander Korotkov
<a.korotkov@postgrespro.ru> wrote:
> On Fri, Dec 21, 2018 at 1:50 AM Alexander Korotkov
> <a.korotkov@postgrespro.ru> wrote:
> > чт, 20 дек. 2018 г., 2:22 Andres Freund andres@anarazel.de:
> >> On 2018-12-03 16:07:40 -0800, Andres Freund wrote:
> >> > As far as I can tell that increase comes laregely from the new GIN
> >> > tests.  Could one of you please look at keeping the test time increase
> >> > to something more reasonable?
> >>
> >> Ping?
> >>
> >> It's also one of the slowest tests outside of valgrind...
> >
> > I'm going to take a look on that.
>
> BTW, patch for reducing isolation testing for gin predicate locking is
> attached.  Could anybody check its execution time with valgrind (and
> compare with execution time of previous test suite)?

I've managed to run it myself.  I've added following lines to
src/test/isolation/Makefile to make it handy running these tests under
valgrind.

ifdef VALGRIND
override with_temp_install += PGCTLTIMEOUT=600 \
    valgrind --leak-check=no --gen-suppressions=all \
    --suppressions=../../tools/valgrind.supp --time-stamp=yes \
    --log-file=pid-%p.log --trace-children=yes \
    --trace-children-skip=*/initdb
endif

With only predicate-gin in isolation_schedule patched version takes
less than minute.

real 0m53.866s
user 0m9.472s
sys 0m1.706s

I also run unpatched version of predicate-gin test, but cancel that
after 30 minutes of waiting.  So, it appears that patch solves the
problem.  I'm going to commit it if no objections.

------
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

On 12/24/18 5:47 PM, Andrew Dunstan wrote:
> On 12/24/18 5:25 PM, Alexander Korotkov wrote:
>> On Fri, Dec 21, 2018 at 1:50 AM Alexander Korotkov
>> <a.korotkov@postgrespro.ru> wrote:
>>> чт, 20 дек. 2018 г., 2:22 Andres Freund andres@anarazel.de:
>>>> On 2018-12-03 16:07:40 -0800, Andres Freund wrote:
>>>>> As far as I can tell that increase comes laregely from the new GIN
>>>>> tests.  Could one of you please look at keeping the test time increase
>>>>> to something more reasonable?
>>>> Ping?
>>>>
>>>> It's also one of the slowest tests outside of valgrind...
>>> I'm going to take a look on that.
>> BTW, patch for reducing isolation testing for gin predicate locking is
>> attached.  Could anybody check its execution time with valgrind (and
>> compare with execution time of previous test suite)?
>
>
> Will try if I get some time to spare in the next 72 hours or so
>
>

The latest test on lousyjack ran with this applied. The isolation tests
went from 285 minutes to 65 minutes.


So +1 for applying this.


cheers


andrew


-- 
Andrew Dunstan                https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On Thu, Dec 27, 2018 at 7:58 PM Andrew Dunstan
<andrew.dunstan@2ndquadrant.com> wrote:
> On 12/24/18 5:47 PM, Andrew Dunstan wrote:
> > On 12/24/18 5:25 PM, Alexander Korotkov wrote:
> >> BTW, patch for reducing isolation testing for gin predicate locking is
> >> attached.  Could anybody check its execution time with valgrind (and
> >> compare with execution time of previous test suite)?
> >
> > Will try if I get some time to spare in the next 72 hours or so
>
> The latest test on lousyjack ran with this applied. The isolation tests
> went from 285 minutes to 65 minutes.
>
> So +1 for applying this.

Thank you very much!
So, pushed.

------
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company