Horton

Thread

Commits

  1. Add minimal test for EXPLAIN ANALYZE of parallel query.

  1. parallel explain analyze support not exercised

    Andres Freund <andres@anarazel.de> — 2017-03-31T18:55:41Z

    Hi,

As visible in [1], the explain analyze codepaths of parallel query isn't
exercised in the tests.  That used to be not entirely trivial if the
output was to be displayed (due to timing), but we should be able to do
that now that we have the SUMMARY option.

E.g.
SET max_parallel_workers = 0;
EXPLAIN (analyze, timing off, summary off, costs off) SELECT * FROM blarg2 WHERE generate_series < 0;
┌───────────────────────────────────────────────────────────┐
│                        QUERY PLAN                         │
├───────────────────────────────────────────────────────────┤
│ Gather (actual rows=0 loops=1)                            │
│   Workers Planned: 10                                     │
│   Workers Launched: 0                                     │
│   ->  Parallel Seq Scan on blarg2 (actual rows=0 loops=1) │
│         Filter: (generate_series < 0)                     │
│         Rows Removed by Filter: 10000000                  │
└───────────────────────────────────────────────────────────┘

should be reproducible.  I'd suggest additionally adding one tests that
throws the EXPLAIN output away, but actually enables paralellism.

Greetings,

Andres Freund

[1] https://coverage.postgresql.org/src/backend/executor/execParallel.c.gcov.html

  2. Re: parallel explain analyze support not exercised

    Rafia Sabih <rafia.sabih@enterprisedb.com> — 2017-04-03T04:56:27Z

    On Sat, Apr 1, 2017 at 12:25 AM, Andres Freund <andres@anarazel.de> wrote:
> Hi,
>
> As visible in [1], the explain analyze codepaths of parallel query isn't
> exercised in the tests.  That used to be not entirely trivial if the
> output was to be displayed (due to timing), but we should be able to do
> that now that we have the SUMMARY option.
>
> E.g.
> SET max_parallel_workers = 0;
> EXPLAIN (analyze, timing off, summary off, costs off) SELECT * FROM blarg2 WHERE generate_series < 0;
> ┌───────────────────────────────────────────────────────────┐
> │                        QUERY PLAN                         │
> ├───────────────────────────────────────────────────────────┤
> │ Gather (actual rows=0 loops=1)                            │
> │   Workers Planned: 10                                     │
> │   Workers Launched: 0                                     │
> │   ->  Parallel Seq Scan on blarg2 (actual rows=0 loops=1) │
> │         Filter: (generate_series < 0)                     │
> │         Rows Removed by Filter: 10000000                  │
> └───────────────────────────────────────────────────────────┘
>
> should be reproducible.  I'd suggest additionally adding one tests that
> throws the EXPLAIN output away, but actually enables paralellism.
>
> Greetings,
>
> Andres Freund
>
> [1] https://coverage.postgresql.org/src/backend/executor/execParallel.c.gcov.html

Please find the attached for the same.

-- 
Regards,
Rafia Sabih
EnterpriseDB: http://www.enterprisedb.com/

  3. Re: parallel explain analyze support not exercised

    Andres Freund <andres@anarazel.de> — 2017-04-03T17:53:48Z

    On 2017-04-03 10:26:27 +0530, Rafia Sabih wrote:
> On Sat, Apr 1, 2017 at 12:25 AM, Andres Freund <andres@anarazel.de> wrote:
> > Hi,
> >
> > As visible in [1], the explain analyze codepaths of parallel query isn't
> > exercised in the tests.  That used to be not entirely trivial if the
> > output was to be displayed (due to timing), but we should be able to do
> > that now that we have the SUMMARY option.
> >
> > E.g.
> > SET max_parallel_workers = 0;
> > EXPLAIN (analyze, timing off, summary off, costs off) SELECT * FROM blarg2 WHERE generate_series < 0;
> > ┌───────────────────────────────────────────────────────────┐
> > │                        QUERY PLAN                         │
> > ├───────────────────────────────────────────────────────────┤
> > │ Gather (actual rows=0 loops=1)                            │
> > │   Workers Planned: 10                                     │
> > │   Workers Launched: 0                                     │
> > │   ->  Parallel Seq Scan on blarg2 (actual rows=0 loops=1) │
> > │         Filter: (generate_series < 0)                     │
> > │         Rows Removed by Filter: 10000000                  │
> > └───────────────────────────────────────────────────────────┘
> >
> > should be reproducible.  I'd suggest additionally adding one tests that
> > throws the EXPLAIN output away, but actually enables paralellism.
> >
> > Greetings,
> >
> > Andres Freund
> >
> > [1] https://coverage.postgresql.org/src/backend/executor/execParallel.c.gcov.html
> 
> Please find the attached for the same.

> +-- to increase the parallel query test coverage
> +EXPLAIN (analyze, timing off, summary off, costs off) SELECT * FROM tenk1;
> +                         QUERY PLAN                          
> +-------------------------------------------------------------
> + Gather (actual rows=10000 loops=1)
> +   Workers Planned: 4
> +   Workers Launched: 4
> +   ->  Parallel Seq Scan on tenk1 (actual rows=2000 loops=5)
> +(4 rows)

Is there an issue that we might not actually be able to start all four
workers?  Serious question, not rhetorical.

Regards,

Andres

  4. Re: parallel explain analyze support not exercised

    Robert Haas <robertmhaas@gmail.com> — 2017-04-03T19:13:13Z

    On Mon, Apr 3, 2017 at 1:53 PM, Andres Freund <andres@anarazel.de> wrote:
>> Please find the attached for the same.
>
>> +-- to increase the parallel query test coverage
>> +EXPLAIN (analyze, timing off, summary off, costs off) SELECT * FROM tenk1;
>> +                         QUERY PLAN
>> +-------------------------------------------------------------
>> + Gather (actual rows=10000 loops=1)
>> +   Workers Planned: 4
>> +   Workers Launched: 4
>> +   ->  Parallel Seq Scan on tenk1 (actual rows=2000 loops=5)
>> +(4 rows)
>
> Is there an issue that we might not actually be able to start all four
> workers?  Serious question, not rhetorical.

If this is 'make check', then we should have 8 parallel workers
allowed, so if we only do one of these at a time, then I think we're
OK.  But if somebody changes that configuration setting or if it's
'make installcheck', then the configuration could be anything.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

  5. Re: parallel explain analyze support not exercised

    Andres Freund <andres@anarazel.de> — 2017-04-03T19:31:59Z

    On 2017-04-03 15:13:13 -0400, Robert Haas wrote:
> On Mon, Apr 3, 2017 at 1:53 PM, Andres Freund <andres@anarazel.de> wrote:
> >> Please find the attached for the same.
> >
> >> +-- to increase the parallel query test coverage
> >> +EXPLAIN (analyze, timing off, summary off, costs off) SELECT * FROM tenk1;
> >> +                         QUERY PLAN
> >> +-------------------------------------------------------------
> >> + Gather (actual rows=10000 loops=1)
> >> +   Workers Planned: 4
> >> +   Workers Launched: 4
> >> +   ->  Parallel Seq Scan on tenk1 (actual rows=2000 loops=5)
> >> +(4 rows)
> >
> > Is there an issue that we might not actually be able to start all four
> > workers?  Serious question, not rhetorical.
> 
> If this is 'make check', then we should have 8 parallel workers
> allowed, so if we only do one of these at a time, then I think we're
> OK.  But if somebody changes that configuration setting or if it's
> 'make installcheck', then the configuration could be anything.

Hm - we already rely on max_parallel_workers_per_gather being set with
some of the explains in the test.  So I guess we're ok also relying on
actual workers being present?

- Andres

  6. Re: parallel explain analyze support not exercised

    Robert Haas <robertmhaas@gmail.com> — 2017-04-03T21:11:33Z

    On Mon, Apr 3, 2017 at 3:31 PM, Andres Freund <andres@anarazel.de> wrote:
>> If this is 'make check', then we should have 8 parallel workers
>> allowed, so if we only do one of these at a time, then I think we're
>> OK.  But if somebody changes that configuration setting or if it's
>> 'make installcheck', then the configuration could be anything.
>
> Hm - we already rely on max_parallel_workers_per_gather being set with
> some of the explains in the test.  So I guess we're ok also relying on
> actual workers being present?

I'm not really sure about that one way or the other.  Our policy on
which configurations are supported vis-a-vis 'make installcheck' seems
to be, essentially, that if a sufficiently-prominent community member
cares about it, then it ends up getting made to work, unless an
even-more-prominent community member objects.  That's why, for
example, our regression tests pass in Czech.  I can't begin to guess
whether breaking installcheck against configurations with low values
of max_parallel_workers or max_worker_processes will bother anybody.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

  7. Re: parallel explain analyze support not exercised

    Andres Freund <andres@anarazel.de> — 2017-04-06T20:50:19Z

    Hi,

On 2017-04-03 17:11:33 -0400, Robert Haas wrote:
> On Mon, Apr 3, 2017 at 3:31 PM, Andres Freund <andres@anarazel.de> wrote:
> >> If this is 'make check', then we should have 8 parallel workers
> >> allowed, so if we only do one of these at a time, then I think we're
> >> OK.  But if somebody changes that configuration setting or if it's
> >> 'make installcheck', then the configuration could be anything.
> >
> > Hm - we already rely on max_parallel_workers_per_gather being set with
> > some of the explains in the test.  So I guess we're ok also relying on
> > actual workers being present?
> 
> I'm not really sure about that one way or the other.  Our policy on
> which configurations are supported vis-a-vis 'make installcheck' seems
> to be, essentially, that if a sufficiently-prominent community member
> cares about it, then it ends up getting made to work, unless an
> even-more-prominent community member objects.  That's why, for
> example, our regression tests pass in Czech.  I can't begin to guess
> whether breaking installcheck against configurations with low values
> of max_parallel_workers or max_worker_processes will bother anybody.

I guess we'll have to see. My personal conclusion is that greater
coverage of parallelism is worth some very minor config trouble for
people doing installcheck against clusters with non-default config.

Thanks Rafia!

- Andres

  8. Re: parallel explain analyze support not exercised

    Tom Lane <tgl@sss.pgh.pa.us> — 2017-04-06T21:33:49Z

    Andres Freund <andres@anarazel.de> writes:
> I guess we'll have to see. My personal conclusion is that greater
> coverage of parallelism is worth some very minor config trouble for
> people doing installcheck against clusters with non-default config.

The buildfarm seems entirely unwilling to play along.

			regards, tom lane

  9. Re: parallel explain analyze support not exercised

    Andres Freund <andres@anarazel.de> — 2017-04-06T21:36:41Z

    On 2017-04-06 17:33:49 -0400, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
> > I guess we'll have to see. My personal conclusion is that greater
> > coverage of parallelism is worth some very minor config trouble for
> > people doing installcheck against clusters with non-default config.
> 
> The buildfarm seems entirely unwilling to play along.

That was the parallel bitmap scan test, not this (I think).  I've
already pushed a fix which should address the issue - it at least does
so locally.  Both Dilip and I had apparently forgotten that we disallow
setting effective_io_concurrency on platforms without USE_PREFETCH
support.

- Andres