Обсуждение: Substantial bloat in postgres_fdw regression test runtime

In 9.6, "make installcheck" in contrib/postgres_fdw takes a shade
under 3 seconds on my machine.  In HEAD, it's taking 10 seconds.
I am not happy, especially not since there's no parallelization
of the contrib regression tests.  That's a direct consumption of
my time and all other developers' time too.  This evidently came
in with commit 7012b132d (Push down aggregates to remote servers),
and while that's a laudable feature, I cannot help thinking that
it does not deserve this much of an imposition on every make check
that's ever done for the rest of eternity.
        regards, tom lane

On Thu, Nov 3, 2016 at 1:58 PM, Jeevan Chalke
<jeevan.chalke@enterprisedb.com> wrote:
>
> On Wed, Nov 2, 2016 at 10:09 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>
>> In 9.6, "make installcheck" in contrib/postgres_fdw takes a shade
>> under 3 seconds on my machine.  In HEAD, it's taking 10 seconds.
>> I am not happy, especially not since there's no parallelization
>> of the contrib regression tests.  That's a direct consumption of
>> my time and all other developers' time too.  This evidently came
>> in with commit 7012b132d (Push down aggregates to remote servers),
>> and while that's a laudable feature, I cannot help thinking that
>> it does not deserve this much of an imposition on every make check
>> that's ever done for the rest of eternity.
>
>
> Thanks Tom for reporting this substantial bloat in postgres_fdw regression
> test runtime. On my machine "make installcheck" in contrib/postgres_fdw
> takes 6.2 seconds on master (HEAD:
> 770671062f130a830aa89100c9aa2d26f8d4bf32).
> However if I remove all tests added for aggregate push down, it drops down
> to 2.2 seconds. Oops 4 seconds more.
>
> I have timed each of my tests added as part of aggregate push down patch and
> observed that one of the test (LATERAL one) is taking around 3.5 seconds.
> This is causing because of the parameterization. I have added a filter so
> that we will have less number of rows for parameterization. Doing this,
> lateral query in question now runs in 100ms. Also updated few more queries
> which were taking more than 100ms to have runtime around 30ms or so. So
> effectively, with changes "make installcheck" now takes around 2.5 seconds.
>
> Attached patch with test-case modification.
>

I verified that this patch indeed bring the time down to 2 to 3
seconds from 10 seconds.

The additional condition t2.c2 = 6 seems to echo the filter t2.c2 = 6
of aggregate. We wouldn't know which of those actually worked. I
modified the testcase to use t2.c2 % 6 = 0 instead and keep the filter
condition intact. This increases the execution time by .2s, which may
be ok. Let me know what you thing of the attached patch.

Also, please add this to the commitfest, so that it isn't forgotten.
-- 
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company

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

Ashutosh Bapat <ashutosh.bapat@enterprisedb.com> writes:
> On Thu, Nov 3, 2016 at 1:58 PM, Jeevan Chalke
> <jeevan.chalke@enterprisedb.com> wrote:
>> Attached patch with test-case modification.

> I verified that this patch indeed bring the time down to 2 to 3
> seconds from 10 seconds.

Thanks for working on this, guys.

> The additional condition t2.c2 = 6 seems to echo the filter t2.c2 = 6
> of aggregate. We wouldn't know which of those actually worked. I
> modified the testcase to use t2.c2 % 6 = 0 instead and keep the filter
> condition intact. This increases the execution time by .2s, which may
> be ok. Let me know what you thing of the attached patch.

Agreed, that seems like a good compromise.  Pushed that way.

> Also, please add this to the commitfest, so that it isn't forgotten.

No need.
        regards, tom lane