Re: [HACKERS] parallel.c is not marked as test covered

On Wed, Aug 17, 2016 at 1:34 AM, Peter Eisentraut wrote: > On 6/20/16 11:16 PM, Tom Lane wrote: >>> > I think this test would only fail if it runs out of workers, and that >>> > would only happen in an installcheck run against a server configured in >>> > a

Re: [HACKERS] parallel.c is not marked as test covered

On 6/20/16 11:16 PM, Tom Lane wrote: >> > I think this test would only fail if it runs out of workers, and that >> > would only happen in an installcheck run against a server configured in >> > a nonstandard way or that is doing something else -- which doesn't >> > happen on the buildfarm. >

Re: [HACKERS] parallel.c is not marked as test covered

On Tue, Aug 2, 2016 at 1:17 PM, Peter Eisentraut wrote: > On 6/19/16 10:00 PM, Robert Haas wrote: >>> Independent of that, it would help testing things like this if we allowed >>> > setting max_worker_processes to 0, instead of the current minimum 1. If >>> >

Re: [HACKERS] parallel.c is not marked as test covered

On 6/19/16 10:00 PM, Robert Haas wrote: >> Independent of that, it would help testing things like this if we allowed >> > setting max_worker_processes to 0, instead of the current minimum 1. If >> > there a reason for the minimum of 1? > I believe that's pure brain fade on my part. I think the

Re: [HACKERS] parallel.c is not marked as test covered

On Tue, Jun 21, 2016 at 1:24 AM, Robert Haas wrote: > > On Mon, Jun 20, 2016 at 1:13 PM, Tom Lane wrote: > > Robert Haas writes: > >> On Sun, Jun 19, 2016 at 10:23 PM, Tom Lane wrote: > >>> Personally, I'm +1

Re: [HACKERS] parallel.c is not marked as test covered

Peter Eisentraut writes: > On 6/19/16 5:55 PM, Tom Lane wrote: >> Depending on what the percentage actually is, maybe we could treat >> this like the "random" test, and allow a failure to be disregarded >> overall? But that doesn't seem very nice either, in view

Re: [HACKERS] parallel.c is not marked as test covered

On 6/19/16 5:55 PM, Tom Lane wrote: Depending on what the percentage actually is, maybe we could treat this like the "random" test, and allow a failure to be disregarded overall? But that doesn't seem very nice either, in view of our increasing reliance on automated testing. If "random" were

Re: [HACKERS] parallel.c is not marked as test covered

Alvaro Herrera writes: > Tom Lane wrote: >> This seems like pretty good evidence that we should remove the "ignored" >> marking for the random test, and maybe remove that functionality from >> pg_regress altogether. We could probably adjust the test to decrease >> its

Re: [HACKERS] parallel.c is not marked as test covered

On Mon, Jun 20, 2016 at 5:47 PM, Robert Haas wrote: > On Mon, Jun 20, 2016 at 4:52 PM, David G. Johnston > wrote: > > Internal or external I do think the number and type of flags described > here, > > for the purposes described, seems

Re: [HACKERS] parallel.c is not marked as test covered

Tom Lane wrote: > This seems like pretty good evidence that we should remove the "ignored" > marking for the random test, and maybe remove that functionality from > pg_regress altogether. We could probably adjust the test to decrease > its risk-of-failure by another factor of ten or so, if

Re: [HACKERS] parallel.c is not marked as test covered

I wrote: > Depending on what the percentage actually is, maybe we could treat > this like the "random" test, and allow a failure to be disregarded > overall? But that doesn't seem very nice either, in view of our > increasing reliance on automated testing. If "random" were failing > 90% of the

Re: [HACKERS] parallel.c is not marked as test covered

On Mon, Jun 20, 2016 at 4:52 PM, David G. Johnston wrote: > Internal or external I do think the number and type of flags described here, > for the purposes described, seems undesirable from an architectural > standpoint. Well, that seems like a bold statement to me,

Re: [HACKERS] parallel.c is not marked as test covered

On Mon, Jun 20, 2016 at 4:03 PM, Robert Haas wrote: > On Mon, Jun 20, 2016 at 3:29 PM, David G. Johnston > wrote: > > The entire theory here looks whacked - and seems to fall into the "GUCs > > controlling results" bucket of undesirable things.

Re: [HACKERS] parallel.c is not marked as test covered

On Mon, Jun 20, 2016 at 3:29 PM, David G. Johnston wrote: > The entire theory here looks whacked - and seems to fall into the "GUCs > controlling results" bucket of undesirable things. As far as I can see, this entire email is totally wrong and off-base, because the

Re: [HACKERS] parallel.c is not marked as test covered

On Mon, Jun 20, 2016 at 1:13 PM, Tom Lane wrote: > Robert Haas writes: >> On Sun, Jun 19, 2016 at 10:23 PM, Tom Lane wrote: >>> Personally, I'm +1 for such tinkering if it makes the feature either more >>> controllable or more

Re: [HACKERS] parallel.c is not marked as test covered

On Mon, Jun 20, 2016 at 12:06 PM, Robert Haas wrote: > On Sun, Jun 19, 2016 at 10:23 PM, Tom Lane wrote: > >> although I fear we > >> might be getting to a level of tinkering with parallel query that > >> starts to look more like feature development. >

Re: [HACKERS] parallel.c is not marked as test covered

Robert Haas writes: > On Sun, Jun 19, 2016 at 10:23 PM, Tom Lane wrote: >> Personally, I'm +1 for such tinkering if it makes the feature either more >> controllable or more understandable. After reading the comments at the >> head of nodeGather.c,

Re: [HACKERS] parallel.c is not marked as test covered

On Mon, Jun 20, 2016 at 1:26 AM, Amit Kapila wrote: > I have done analysis on this and didn't found any use case where passing > CURSOR_OPT_PARALLEL_OK in exec_stmt_execsql() can help in parallelizing the > queries. Basically, there seems to be three ways in which

Re: [HACKERS] parallel.c is not marked as test covered

On Sun, Jun 19, 2016 at 10:23 PM, Tom Lane wrote: >> although I fear we >> might be getting to a level of tinkering with parallel query that >> starts to look more like feature development. > > Personally, I'm +1 for such tinkering if it makes the feature either more >

Re: [HACKERS] parallel.c is not marked as test covered

On Thu, Jun 16, 2016 at 8:20 AM, Robert Haas wrote: > > On Wed, Jun 15, 2016 at 10:48 PM, Amit Kapila wrote: > > exec_stmt_execsql() is used to execute SQL statements insider plpgsql which > > includes dml statements as well, so probably you wanted

Re: [HACKERS] parallel.c is not marked as test covered

Robert Haas writes: > On Sun, Jun 19, 2016 at 5:22 PM, Peter Eisentraut > wrote: >> Well, the purpose of the test is to check the error passing between worker >> and leader. If we just silently revert to not doing that, then we can't >>

Re: [HACKERS] parallel.c is not marked as test covered

On Sun, Jun 19, 2016 at 5:22 PM, Peter Eisentraut wrote: > Well, the purpose of the test is to check the error passing between worker > and leader. If we just silently revert to not doing that, then we can't > really be sure that we're testing the right thing.

Re: [HACKERS] parallel.c is not marked as test covered

Peter Eisentraut writes: > On 6/19/16 3:09 PM, Robert Haas wrote: >> On Sun, Jun 19, 2016 at 11:36 AM, Tom Lane wrote: >>> No, it *might* execute in a worker. If you can get one. >> Right. > Well, the purpose of the test is to check the

Re: [HACKERS] parallel.c is not marked as test covered

On 6/19/16 3:09 PM, Robert Haas wrote: On Sun, Jun 19, 2016 at 11:36 AM, Tom Lane wrote: Amit Kapila writes: On Sun, Jun 19, 2016 at 10:10 AM, Tom Lane wrote: Would this not result in unstable test output depending on whether

Re: [HACKERS] parallel.c is not marked as test covered

On Sun, Jun 19, 2016 at 11:36 AM, Tom Lane wrote: > Amit Kapila writes: >> On Sun, Jun 19, 2016 at 10:10 AM, Tom Lane wrote: >>> Would this not result in unstable test output depending on whether the >>> code executes in the

Re: [HACKERS] parallel.c is not marked as test covered

Amit Kapila writes: > On Sun, Jun 19, 2016 at 10:10 AM, Tom Lane wrote: >> Would this not result in unstable test output depending on whether the >> code executes in the leader or a worker? > Before doing that test, we set force_parallel_mode=1, so

Re: [HACKERS] parallel.c is not marked as test covered

On Sun, Jun 19, 2016 at 10:10 AM, Tom Lane wrote: > > Peter Eisentraut writes: > > With that, I think it would be preferable to undo the context-hiding > > dance in the tests, as in the attached patch, no? > > Would this not result in

Re: [HACKERS] parallel.c is not marked as test covered

Peter Eisentraut writes: > With that, I think it would be preferable to undo the context-hiding > dance in the tests, as in the attached patch, no? Would this not result in unstable test output depending on whether the code executes in the leader or a worker?

Re: [HACKERS] parallel.c is not marked as test covered

On 6/17/16 9:26 AM, Robert Haas wrote: On Wed, Jun 15, 2016 at 5:10 PM, Robert Haas wrote: I'm comfortable with that. Feel free to make it so, unless you want me to do it for some reason. Time is short, so I did this. With that, I think it would be preferable to

Re: [HACKERS] parallel.c is not marked as test covered

On Wed, Jun 15, 2016 at 5:10 PM, Robert Haas wrote: > I'm comfortable with that. Feel free to make it so, unless you want > me to do it for some reason. Time is short, so I did this. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL

Re: [HACKERS] parallel.c is not marked as test covered

On Wed, Jun 15, 2016 at 10:48 PM, Amit Kapila wrote: > exec_stmt_execsql() is used to execute SQL statements insider plpgsql which > includes dml statements as well, so probably you wanted to play safe by not > allowing parallel option from that place. However, I think

Re: [HACKERS] parallel.c is not marked as test covered

On Thu, Jun 16, 2016 at 12:46 AM, Robert Haas wrote: > > On Wed, Jun 15, 2016 at 5:23 AM, Amit Kapila wrote: > >> > Considering above analysis is correct, we have below options: > >> > a. Modify the test such that it actually generates an error and

Re: [HACKERS] parallel.c is not marked as test covered

On Wed, Jun 15, 2016 at 4:59 PM, Peter Eisentraut wrote: > On 6/14/16 12:37 PM, Robert Haas wrote: >> ERROR: can't generate random numbers because you haven't specified a seed >> >> ...to which the user will reply, "oh yes I did; in fact I ran SELECT >>

Re: [HACKERS] parallel.c is not marked as test covered

On 6/14/16 12:37 PM, Robert Haas wrote: ERROR: can't generate random numbers because you haven't specified a seed ...to which the user will reply, "oh yes I did; in fact I ran SELECT magic_setseed(42) just before I ran the offending query!". They'll then contact an expert (hopefully) who will

Re: [HACKERS] parallel.c is not marked as test covered

On Wed, Jun 15, 2016 at 5:23 AM, Amit Kapila wrote: >> > Considering above analysis is correct, we have below options: >> > a. Modify the test such that it actually generates an error and to hide >> > the >> > context, we can exception block and raise some generic error.

Re: [HACKERS] parallel.c is not marked as test covered

On Wed, Jun 15, 2016 at 12:07 PM, Noah Misch wrote: > > On Wed, Jun 15, 2016 at 11:50:33AM +0530, Amit Kapila wrote: > > In short, this test doesn't serve it's purpose which is to generate an > > error from worker. > > That's bad. Thanks for figuring out the problem. > > > do

Re: [HACKERS] parallel.c is not marked as test covered

On Wed, Jun 15, 2016 at 12:07 PM, Noah Misch wrote: > > On Wed, Jun 15, 2016 at 11:50:33AM +0530, Amit Kapila wrote: > > do $$begin > > Perform stringu1::int2 from tenk1 where unique1 = 1; > > end$$; > > > > ERROR: invalid input syntax for integer: "BA" > > CONTEXT:

Re: [HACKERS] parallel.c is not marked as test covered

On Wed, Jun 15, 2016 at 11:50:33AM +0530, Amit Kapila wrote: > In short, this test doesn't serve it's purpose which is to generate an > error from worker. That's bad. Thanks for figuring out the problem. > do $$begin > Perform stringu1::int2 from tenk1 where unique1 = 1; > end$$; > > ERROR:

Re: [HACKERS] parallel.c is not marked as test covered

On Wed, Jun 15, 2016 at 1:42 AM, Robert Haas wrote: > > On Tue, Jun 14, 2016 at 1:14 PM, Tom Lane wrote: > > > > I have not dug into the code enough to find out exactly what's happening > > in Peter's complaint, but it seems like it would be a good idea

Re: [HACKERS] parallel.c is not marked as test covered

On Tue, Jun 14, 2016 at 1:14 PM, Tom Lane wrote: > Robert Haas writes: >> On Tue, Jun 14, 2016 at 12:51 PM, Tom Lane wrote: >>> FWIW, I follow all of your reasoning except this. If we believe that the >>> parallel worker context

Re: [HACKERS] parallel.c is not marked as test covered

Robert Haas writes: > On Tue, Jun 14, 2016 at 12:51 PM, Tom Lane wrote: >> FWIW, I follow all of your reasoning except this. If we believe that the >> parallel worker context line is useful, then it is a bug that plpgsql >> suppresses it. If we don't

Re: [HACKERS] parallel.c is not marked as test covered

On Tue, Jun 14, 2016 at 12:51 PM, Tom Lane wrote: > Robert Haas writes: >> On Fri, Jun 10, 2016 at 4:12 PM, Robert Haas wrote: >>> On Fri, Jun 10, 2016 at 1:49 PM, Peter Eisentraut >>> wrote:

Re: [HACKERS] parallel.c is not marked as test covered

Robert Haas writes: > On Fri, Jun 10, 2016 at 4:12 PM, Robert Haas wrote: >> On Fri, Jun 10, 2016 at 1:49 PM, Peter Eisentraut >> wrote: >>> Elsewhere in this thread I suggested getting rid of the parallel worker

Re: [HACKERS] parallel.c is not marked as test covered

On Fri, Jun 10, 2016 at 4:12 PM, Robert Haas wrote: > On Fri, Jun 10, 2016 at 1:49 PM, Peter Eisentraut > wrote: >> Regarding the patch that ended up being committed, I wonder if it is >> intentional that PL/pgSQL overwrites the context

Re: [HACKERS] parallel.c is not marked as test covered

On Fri, Jun 10, 2016 at 1:49 PM, Peter Eisentraut wrote: > Regarding the patch that ended up being committed, I wonder if it is > intentional that PL/pgSQL overwrites the context from the parallel worker. > Shouldn't the context effectively look like > > ERROR:

Re: [HACKERS] parallel.c is not marked as test covered

On 6/7/16 11:43 PM, Noah Misch wrote: 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. Regarding the patch that ended up being committed, I wonder if it is intentional that PL/pgSQL

Re: [HACKERS] parallel.c is not marked as test covered

On Tue, Jun 7, 2016 at 11:43 PM, Noah Misch wrote: > Committed that way. Thanks for the carefully-considered commit, Noah. And thanks Clement, Peter, and others involved in figuring out the best way to do this and drawing attention to it. -- Robert Haas EnterpriseDB:

Re: [HACKERS] parallel.c is not marked as test covered

On Tue, Jun 07, 2016 at 10:24:33PM +, 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.gb29...@momjian.us) so > I

Re: [HACKERS] parallel.c is not marked as test covered

> > On Sun, May 15, 2016 at 12:53:13PM +, Clément Prévost wrote: > > After some experiments, I found out that, for my setup (9b7bfc3a88ef7b), > a > > parallel seq scan is used given both parallel_setup_cost > > and parallel_tuple_cost are set to 0 and given that the table is at > least 3 > >

Re: [HACKERS] parallel.c is not marked as test covered

On 6/7/16 1:27 AM, Noah Misch wrote: Testing under these conditions does not test the planner job at all but at least some parallel code can be run on the build farm and the test suite gets 2643 more lines and 188 more function covered. Nice. Please see also my message

Re: [HACKERS] parallel.c is not marked as test covered

Thanks for this patch. I have reviewed it: On Sun, May 15, 2016 at 12:53:13PM +, Clément Prévost wrote: > After some experiments, I found out that, for my setup (9b7bfc3a88ef7b), a > parallel seq scan is used given both parallel_setup_cost > and parallel_tuple_cost are set to 0 and given

Re: [HACKERS] parallel.c is not marked as test covered

On Sun, May 29, 2016 at 01:31:24AM -0400, Noah Misch wrote: > On Sun, May 15, 2016 at 12:53:13PM +, Clément Prévost wrote: > > On Mon, May 9, 2016 at 4:50 PM Andres Freund wrote: > > > I think it's a good idea to run a force-parallel run on some buildfarm > > > members.

Re: [HACKERS] parallel.c is not marked as test covered

On 5/9/16 10:50 AM, Andres Freund wrote: I think it's a good idea to run a force-parallel run on some buildfarm members. But I'm rather convinced that the core tests run by all animals need some minimal coverage of parallel queries. Both because otherwise it'll be hard to get some coverage of

Re: [HACKERS] parallel.c is not marked as test covered

On Sun, May 15, 2016 at 12:53:13PM +, Clément Prévost wrote: > On Mon, May 9, 2016 at 4:50 PM Andres Freund wrote: > > I think it's a good idea to run a force-parallel run on some buildfarm > > members. But I'm rather convinced that the core tests run by all animals > >

Re: [HACKERS] parallel.c is not marked as test covered

On Mon, May 9, 2016 at 4:50 PM Andres Freund wrote: > I think it's a good idea to run a force-parallel run on some buildfarm > members. But I'm rather convinced that the core tests run by all animals > need some minimal coverage of parallel queries. Both because otherwise >

Re: [HACKERS] parallel.c is not marked as test covered

On 12 May 2016 at 07:04, Robert Haas wrote: > On Wed, May 11, 2016 at 1:57 PM, Robert Haas wrote: >> I don't immediately understand what's going wrong here. It looks to >> me like make_group_input_target() already called, and that worked OK, >> but

Re: [HACKERS] parallel.c is not marked as test covered

On Wed, May 11, 2016 at 1:57 PM, Robert Haas wrote: > I don't immediately understand what's going wrong here. It looks to > me like make_group_input_target() already called, and that worked OK, > but now make_partialgroup_input_target() is failing using more-or-less > the

Re: [HACKERS] parallel.c is not marked as test covered

On Wed, May 11, 2016 at 1:48 PM, David G. Johnston wrote: > What happens when there are no workers available due to max_worker_processes > already being assigned? Then the leader runs the plan after all. > Related question, if max_parallel_degree is >1 and "the

Re: [HACKERS] parallel.c is not marked as test covered

On Wed, May 11, 2016 at 1:38 PM, Robert Haas wrote: >> I would just go fix this, along the lines of >> >> *** create_plain_partial_paths(PlannerInfo * >> *** 702,708 >> * with all of its inheritance siblings it may well pay off. >> */ >>

Re: [HACKERS] parallel.c is not marked as test covered

On Wed, May 11, 2016 at 10:38 AM, Robert Haas wrote: > On Wed, May 11, 2016 at 12:34 AM, Tom Lane wrote: > >> Hmm, that is strange. I would have expected that to stuff a Gather on > >> top of the Aggregate. I wonder why it's not doing that. > > > >

Re: [HACKERS] parallel.c is not marked as test covered

On Wed, May 11, 2016 at 12:34 AM, Tom Lane wrote: >> Hmm, that is strange. I would have expected that to stuff a Gather on >> top of the Aggregate. I wonder why it's not doing that. > > The reason is that create_plain_partial_paths() contains a hard-wired > decision not to

Re: [HACKERS] parallel.c is not marked as test covered

Robert Haas writes: > On Mon, May 9, 2016 at 11:11 AM, Tom Lane wrote: >> regression=# set force_parallel_mode TO on; >> SET >> regression=# explain select count(*) from tenk1; >> QUERY PLAN >>

Re: [HACKERS] parallel.c is not marked as test covered

On Mon, May 9, 2016 at 11:11 AM, Tom Lane wrote: > Andres Freund writes: >> I think it's a good idea to run a force-parallel run on some buildfarm >> members. > > Noah's already doing that on at least one of his critters. But some more > wouldn't hurt. I

Re: [HACKERS] parallel.c is not marked as test covered

Andres Freund writes: > I think it's a good idea to run a force-parallel run on some buildfarm > members. Noah's already doing that on at least one of his critters. But some more wouldn't hurt. > But I'm rather convinced that the core tests run by all animals > need some

Re: [HACKERS] parallel.c is not marked as test covered

On 2016-05-08 22:20:55 -0300, Alvaro Herrera wrote: > David Rowley wrote: > > > I'm not entirely sure which machine generates that coverage output, > > but the problem with it is that it's just one machine. We do have at > > least one buildfarm member which runs with force_parallel_mode = > >

Re: [HACKERS] parallel.c is not marked as test covered

On 9 May 2016 at 14:26, Alvaro Herrera wrote: > David Rowley wrote: >> On 9 May 2016 at 13:20, Alvaro Herrera wrote: > >> > It's not a buildfarm machine, but a machine setup specifically for >> > coverage reports. It runs "make check-world"

Re: [HACKERS] parallel.c is not marked as test covered

David Rowley wrote: > On 9 May 2016 at 13:20, Alvaro Herrera wrote: > > It's not a buildfarm machine, but a machine setup specifically for > > coverage reports. It runs "make check-world" only. I can add some > > additional command(s) to run, if somebody can suggest

Re: [HACKERS] parallel.c is not marked as test covered

On 9 May 2016 at 13:20, Alvaro Herrera wrote: > David Rowley wrote: > >> I'm not entirely sure which machine generates that coverage output, >> but the problem with it is that it's just one machine. We do have at >> least one buildfarm member which runs with

Re: [HACKERS] parallel.c is not marked as test covered

David Rowley wrote: > I'm not entirely sure which machine generates that coverage output, > but the problem with it is that it's just one machine. We do have at > least one buildfarm member which runs with force_parallel_mode = > regress. It's not a buildfarm machine, but a machine setup

Re: [HACKERS] parallel.c is not marked as test covered

On 9 May 2016 at 09:12, Clément Prévost wrote: > The entire parallel.c reported test coverage is zero: > http://coverage.postgresql.org/src/backend/access/transam/parallel.c.gcov.html > > It seem that it's not covered by the original 924bcf4f commit but I don't > know if