Re: pgsql: Support Parallel Append plan nodes.
От | amul sul |
---|---|
Тема | Re: pgsql: Support Parallel Append plan nodes. |
Дата | |
Msg-id | CAAJ_b95zsdBT2jNJuvs6GL3g29EXvSO+q4pxpuQ5mAhf5DTSBw@mail.gmail.com обсуждение исходный текст |
Ответ на | Re: pgsql: Support Parallel Append plan nodes. (amul sul <sulamul@gmail.com>) |
Ответы |
Re: pgsql: Support Parallel Append plan nodes.
|
Список | pgsql-committers |
Copying & reverting to Amit Khandekar's email here: On Wed, Dec 6, 2017 at 11:45 AM, amul sul <sulamul@gmail.com> wrote: >> Thanks Tom for the crash analysis, I think this is the same reason that >> Rajkumar's reported case[1] was crashing only with partition-wise-join = on. >> I tried to fix this crash[2] but missed the place where I have added assert >> check in the assumption that we always coming from the previous check in the >> while loop. >> >> Instead, assert check we need a similar bailout logic[2] before looping back to >> first partial plan. Attached patch does the same, I've tested with >> parallel_leader_participation = off setting as suggested by Andres, make check >> looks good except there is some obvious regression diff. >> >> 1] https://postgr.es/m/CAKcux6m+6nTO6C08kKaj-Waffvgvp-9SD3RCGStX=Mzk0gQU8g@mail.gmail.com >> 2] https://postgr.es/m/CAAJ_b975k58H+Ed4=p0vbJunwO2reOMX5CVB8_R=JmXxY3uW=Q@mail.gmail.com >> > > @@ -506,7 +506,14 @@ choose_next_subplan_for_worker(AppendState *node) > node->as_whichplan = pstate->pa_next_plan++; > if (pstate->pa_next_plan >= node->as_nplans) > { > - Assert(append->first_partial_plan < node->as_nplans); > + /* No partial plans then bail out. */ > + if (append->first_partial_plan >= node->as_nplans) > + { > + pstate->pa_next_plan = INVALID_SUBPLAN_INDEX; > + node->as_whichplan = INVALID_SUBPLAN_INDEX; > + LWLockRelease(&pstate->pa_lock); > + return false; > + } > pstate->pa_next_plan = append->first_partial_plan; > > In the above code, the fact that we have not bailed out from the > earlier for loop means that we have already found an unfinished plan > and node->as_whichplan is set to that plan. So while setting the next > plan above for the other workers to pick, we should not return false, > nor should we set node->as_whichplan to INVALID_SUBPLAN_INDEX. > Instead, just set pa_next_plan to INVALID_SUBPLAN_INDEX. Otherwise, > the chosen plan won't get executed at all. > Understood, thanks for the review. Updated patch attached. 1] https://postgr.es/m/CAJ3gD9e3_D3fFqzWBFYoaF-6yCXgbOFn3Mb-pgd_mxvjpsw7Rw@mail.gmail.com Regards, Amul
Вложения
В списке pgsql-committers по дате отправления: