Re: parallel.c is not marked as test covered - Mailing list pgsql-hackers

From Noah Misch
Subject Re: parallel.c is not marked as test covered
Date
Msg-id 20160608034342.GA804701@tornado.leadboat.com
Whole thread Raw
In response to Re: parallel.c is not marked as test covered  (Clément Prévost <prevostclement@gmail.com>)
Responses Re: parallel.c is not marked as test covered
Re: parallel.c is not marked as test covered
List 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
aggregatestransactions 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
aggregatestransactions 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



pgsql-hackers by date:

Previous
From: Amit Kapila
Date:
Subject: Re: Reviewing freeze map code
Next
From: Amit Kapila
Date:
Subject: Re: Reviewing freeze map code