Re: parallel.c is not marked as test covered

From: Noah Misch <noah(at)leadboat(dot)com>
To: Clément Prévost <prevostclement(at)gmail(dot)com>
Cc: Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, David Rowley <david(dot)rowley(at)2ndquadrant(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: parallel.c is not marked as test covered
Date: 2016-06-08 03:43:42
Message-ID: 20160608034342.GA804701@tornado.leadboat.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Jun 07, 2016 at 10:24:33PM +0000, Clément Prévost wrote:
> I also considered setting max_parallel_degree to 1 to make the test more
> futur-proof but there is a rather long discussion on the setting name (
> https://www.postgresql.org/message-id/20160424035859.GB29404@momjian.us) so
> I can't decide on my own if it's worth the explicit dependency.

It wouldn't be a problem to update this test when renaming the setting, but I
didn't see an impending need to use that setting.

> > As of today, "make installcheck" passes with
> > "default_transaction_isolation =
> > serializable" in postgresql.conf. Let's preserve that property. You could
> > wrap the parallel queries in "begin isolation level repeatable read;"
> > ... "commit;", or you could SET default_transaction_isolation itself.
> >
>
> I did add the transaction, but I don't get why this specific test should
> use this specific transaction isolation level.

We disable all parallelism at serializable isolation. Any other isolation
level would have worked; repeatable read was an arbitrary choice. I added a
comment to that effect.

> -test: select_into select_distinct select_distinct_on select_implicit select_having subselect union case join aggregates transactions random portals arrays btree_index hash_index update namespace prepared_xacts delete
> +test: select_into select_distinct select_distinct_on select_implicit select_having subselect union case join aggregates transactions random portals arrays btree_index hash_index update namespace prepared_xacts delete select_parallel

I moved the test to a different group, in light of this parallel_schedule
header comment:

# By convention, we put no more than twenty tests in any one parallel group;
# this limits the number of connections needed to run the tests.

> + exception
> + -- raise custom exception, the original message contains
> + -- a worker PID that must be hidden in the test output
> + when others then raise exception 'Error in worker';

I changed this to keep the main message while overwriting the CONTEXT; a bug
in this area could very well produce some other error rather than no error.

Committed that way.

Thanks,
nm

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Kapila 2016-06-08 03:47:44 Re: Reviewing freeze map code
Previous Message Amit Kapila 2016-06-08 03:19:35 Re: Reviewing freeze map code