Re: [HACKERS] crashes due to setting max_parallel_workers=0

2017-03-31 Thread Robert Haas
On Tue, Mar 28, 2017 at 11:08 PM, Tomas Vondra wrote: > Maybe. It depends on how valuable it's to keep Gather and GatherMerge > similar - having nreaders in one and not the other seems a bit weird. But > maybe the refactoring would remove it from both nodes? Yeah, it appears to be the case that b

Re: [HACKERS] crashes due to setting max_parallel_workers=0

2017-03-30 Thread Rushabh Lathia
On Wed, Mar 29, 2017 at 8:38 AM, Tomas Vondra wrote: > Hi, > > On 03/28/2017 11:07 AM, Rushabh Lathia wrote: > >> ... >> I think we all agree that we should get rid of nreaders from the >> GatherMergeState and need to do some code re-factor. But if I >> understood correctly that Robert's concern

Re: [HACKERS] crashes due to setting max_parallel_workers=0

2017-03-28 Thread Tomas Vondra
Hi, On 03/28/2017 11:07 AM, Rushabh Lathia wrote: ... I think we all agree that we should get rid of nreaders from the GatherMergeState and need to do some code re-factor. But if I understood correctly that Robert's concern was to do that re-factor as separate commit. Maybe. It depends on

Re: [HACKERS] crashes due to setting max_parallel_workers=0

2017-03-28 Thread Rushabh Lathia
On Tue, Mar 28, 2017 at 10:00 AM, Tomas Vondra wrote: > > > On 03/27/2017 01:40 PM, Rushabh Lathia wrote: > >> >> ... >> I was doing more testing with the patch and I found one more server >> crash with the patch around same area, when we forced the gather >> merge for the scan having zero rows.

Re: [HACKERS] crashes due to setting max_parallel_workers=0

2017-03-27 Thread Tomas Vondra
On 03/27/2017 01:40 PM, Rushabh Lathia wrote: ... I was doing more testing with the patch and I found one more server crash with the patch around same area, when we forced the gather merge for the scan having zero rows. create table dept ( deptno numeric, dname varchar(20); set parallel_tuple

Re: [HACKERS] crashes due to setting max_parallel_workers=0

2017-03-27 Thread Tomas Vondra
On 03/27/2017 05:51 PM, Robert Haas wrote: On Mon, Mar 27, 2017 at 9:54 AM, Tom Lane wrote: Robert Haas writes: On Mon, Mar 27, 2017 at 1:29 AM, Rushabh Lathia wrote: But it seems a bit futile to produce the parallel plan in the first place, because with max_parallel_workers=0 we can't po

Re: [HACKERS] crashes due to setting max_parallel_workers=0

2017-03-27 Thread Robert Haas
On Mon, Mar 27, 2017 at 12:36 PM, Rushabh Lathia wrote: > Hmm I agree that it's good idea, and I will work on that as separate patch. Maybe you want to start with what David already posted? >> Possibly we >> should fix the crash bug first, though, and then do that afterwards. >> What bugs me a l

Re: [HACKERS] crashes due to setting max_parallel_workers=0

2017-03-27 Thread Rushabh Lathia
On Mon, Mar 27, 2017 at 9:52 PM, Robert Haas wrote: > On Mon, Mar 27, 2017 at 12:11 PM, David Rowley > wrote: > > On 28 March 2017 at 04:57, Robert Haas wrote: > >> On Sat, Mar 25, 2017 at 12:18 PM, Rushabh Lathia > >> wrote: > >>> About the original issue reported by Tomas, I did more debuggi

Re: [HACKERS] crashes due to setting max_parallel_workers=0

2017-03-27 Thread Robert Haas
On Mon, Mar 27, 2017 at 12:26 PM, Tom Lane wrote: > Robert Haas writes: >> On Mon, Mar 27, 2017 at 9:54 AM, Tom Lane wrote: >>> Since this has now come up twice, I suggest adding a comment there >>> that explains why we're intentionally ignoring max_parallel_workers. > >> Good idea. How about t

Re: [HACKERS] crashes due to setting max_parallel_workers=0

2017-03-27 Thread Tom Lane
Robert Haas writes: > On Mon, Mar 27, 2017 at 9:54 AM, Tom Lane wrote: >> Since this has now come up twice, I suggest adding a comment there >> that explains why we're intentionally ignoring max_parallel_workers. > Good idea. How about the attached? WFM ... but seems like there should be some

Re: [HACKERS] crashes due to setting max_parallel_workers=0

2017-03-27 Thread Robert Haas
On Mon, Mar 27, 2017 at 12:11 PM, David Rowley wrote: > On 28 March 2017 at 04:57, Robert Haas wrote: >> On Sat, Mar 25, 2017 at 12:18 PM, Rushabh Lathia >> wrote: >>> About the original issue reported by Tomas, I did more debugging and >>> found that - problem was gather_merge_clear_slots() was

Re: [HACKERS] crashes due to setting max_parallel_workers=0

2017-03-27 Thread David Rowley
On 28 March 2017 at 04:57, Robert Haas wrote: > On Sat, Mar 25, 2017 at 12:18 PM, Rushabh Lathia > wrote: >> About the original issue reported by Tomas, I did more debugging and >> found that - problem was gather_merge_clear_slots() was not returning >> the clear slot when nreader is zero (means

Re: [HACKERS] crashes due to setting max_parallel_workers=0

2017-03-27 Thread Robert Haas
On Sat, Mar 25, 2017 at 12:18 PM, Rushabh Lathia wrote: > About the original issue reported by Tomas, I did more debugging and > found that - problem was gather_merge_clear_slots() was not returning > the clear slot when nreader is zero (means nworkers_launched = 0). > Due to the same scan was con

Re: [HACKERS] crashes due to setting max_parallel_workers=0

2017-03-27 Thread Robert Haas
On Mon, Mar 27, 2017 at 9:54 AM, Tom Lane wrote: > Robert Haas writes: >> On Mon, Mar 27, 2017 at 1:29 AM, Rushabh Lathia >> wrote: >>> But it seems a bit futile to produce the parallel plan in the first place, >>> because with max_parallel_workers=0 we can't possibly get any parallel >>> worker

Re: [HACKERS] crashes due to setting max_parallel_workers=0

2017-03-27 Thread Tom Lane
Robert Haas writes: > On Mon, Mar 27, 2017 at 1:29 AM, Rushabh Lathia > wrote: >> But it seems a bit futile to produce the parallel plan in the first place, >> because with max_parallel_workers=0 we can't possibly get any parallel >> workers ever. I wonder why compute_parallel_worker() only looks

Re: [HACKERS] crashes due to setting max_parallel_workers=0

2017-03-27 Thread Robert Haas
On Mon, Mar 27, 2017 at 1:29 AM, Rushabh Lathia wrote: > >> But it seems a bit futile to produce the parallel plan in the first place, >> because with max_parallel_workers=0 we can't possibly get any parallel >> workers ever. I wonder why compute_parallel_worker() only looks at >> max_parallel_wor

Re: [HACKERS] crashes due to setting max_parallel_workers=0

2017-03-27 Thread Rushabh Lathia
On Mon, Mar 27, 2017 at 10:59 AM, Rushabh Lathia wrote: > > > On Mon, Mar 27, 2017 at 3:43 AM, Tomas Vondra < > tomas.von...@2ndquadrant.com> wrote: > >> On 03/25/2017 05:18 PM, Rushabh Lathia wrote: >> >>> >>> >>> On Sat, Mar 25, 2017 at 7:01 PM, Peter Eisentraut >>> >>

Re: [HACKERS] crashes due to setting max_parallel_workers=0

2017-03-26 Thread Rushabh Lathia
On Mon, Mar 27, 2017 at 3:43 AM, Tomas Vondra wrote: > On 03/25/2017 05:18 PM, Rushabh Lathia wrote: > >> >> >> On Sat, Mar 25, 2017 at 7:01 PM, Peter Eisentraut >> > > wrote: >> >> On 3/25/17 09:01, David Rowley wrote: >> > On 25 March 2017 at 23:

Re: [HACKERS] crashes due to setting max_parallel_workers=0

2017-03-26 Thread David Rowley
On 27 March 2017 at 10:23, Tomas Vondra wrote: > I'm not sure we need to invent a new magic value, though. Can we simply look > at force_parallel_mode, and if it's 'regress' then tread 0 differently? see standard_planner() if (force_parallel_mode != FORCE_PARALLEL_OFF && best_path->parallel_safe

Re: [HACKERS] crashes due to setting max_parallel_workers=0

2017-03-26 Thread Tomas Vondra
On 03/25/2017 02:01 PM, David Rowley wrote: > I wondered if there's anything we can do here to better test cases when no workers are able to try to ensure the parallel nodes work correctly, but the more I think about it, the more I don't see wrong with just using SET max_parallel_workers = 0;

Re: [HACKERS] crashes due to setting max_parallel_workers=0

2017-03-26 Thread Tomas Vondra
On 03/25/2017 05:18 PM, Rushabh Lathia wrote: On Sat, Mar 25, 2017 at 7:01 PM, Peter Eisentraut mailto:peter.eisentr...@2ndquadrant.com>> wrote: On 3/25/17 09:01, David Rowley wrote: > On 25 March 2017 at 23:09, Rushabh Lathia mailto:rushabh.lat...@gmail.com>> wrote: >> Also anothe

Re: [HACKERS] crashes due to setting max_parallel_workers=0

2017-03-25 Thread Rushabh Lathia
On Sat, Mar 25, 2017 at 7:01 PM, Peter Eisentraut < peter.eisentr...@2ndquadrant.com> wrote: > On 3/25/17 09:01, David Rowley wrote: > > On 25 March 2017 at 23:09, Rushabh Lathia > wrote: > >> Also another point which I think we should fix is, when someone set > >> max_parallel_workers = 0, we sh

Re: [HACKERS] crashes due to setting max_parallel_workers=0

2017-03-25 Thread Peter Eisentraut
On 3/25/17 09:01, David Rowley wrote: > On 25 March 2017 at 23:09, Rushabh Lathia wrote: >> Also another point which I think we should fix is, when someone set >> max_parallel_workers = 0, we should also set the >> max_parallel_workers_per_gather >> to zero. So that way it we can avoid generating

Re: [HACKERS] crashes due to setting max_parallel_workers=0

2017-03-25 Thread David Rowley
On 26 March 2017 at 00:17, Amit Kapila wrote: > On Sat, Mar 25, 2017 at 6:31 PM, David Rowley > wrote: >> On 25 March 2017 at 23:09, Rushabh Lathia wrote: >>> Also another point which I think we should fix is, when someone set >>> max_parallel_workers = 0, we should also set the >>> max_parallel

Re: [HACKERS] crashes due to setting max_parallel_workers=0

2017-03-25 Thread Amit Kapila
On Sat, Mar 25, 2017 at 6:31 PM, David Rowley wrote: > On 25 March 2017 at 23:09, Rushabh Lathia wrote: >> Also another point which I think we should fix is, when someone set >> max_parallel_workers = 0, we should also set the >> max_parallel_workers_per_gather >> to zero. So that way it we can a

Re: [HACKERS] crashes due to setting max_parallel_workers=0

2017-03-25 Thread David Rowley
On 25 March 2017 at 23:09, Rushabh Lathia wrote: > Also another point which I think we should fix is, when someone set > max_parallel_workers = 0, we should also set the > max_parallel_workers_per_gather > to zero. So that way it we can avoid generating the gather path with > max_parallel_worker =

Re: [HACKERS] crashes due to setting max_parallel_workers=0

2017-03-25 Thread Rushabh Lathia
Thanks Tomas for reporting issue and Thanks David for working on this. I can see the problem in GatherMerge, specially when nworkers_launched is zero. I will look into this issue and will post a fix for the same. Also another point which I think we should fix is, when someone set max_parallel_wor

Re: [HACKERS] crashes due to setting max_parallel_workers=0

2017-03-25 Thread David Rowley
On 25 March 2017 at 13:10, Tomas Vondra wrote: > while working on a patch I ran into some crashes that seem to be caused by > inconsistent handling of max_parallel_workers - queries still seem to be > planned with parallel plans enabled, but then crash at execution. > > The attached script reprodu

Re: [HACKERS] crashes due to setting max_parallel_workers=0

2017-03-24 Thread Amit Kapila
On Sat, Mar 25, 2017 at 7:40 AM, Tomas Vondra wrote: > Hi, > > while working on a patch I ran into some crashes that seem to be caused by > inconsistent handling of max_parallel_workers - queries still seem to be > planned with parallel plans enabled, but then crash at execution. > > The attached

[HACKERS] crashes due to setting max_parallel_workers=0

2017-03-24 Thread Tomas Vondra
Hi, while working on a patch I ran into some crashes that seem to be caused by inconsistent handling of max_parallel_workers - queries still seem to be planned with parallel plans enabled, but then crash at execution. The attached script reproduces the issue on a simple query, causing crashe