Обсуждение: parallel_safe
Hi,
In the comments of add_partial_path, we have:
* We don't generate parameterized partial paths for several reasons. Most
* importantly, they're not safe to execute, because there's nothing to
* make sure that a parallel scan within the parameterized portion of the
* plan is running with the same value in every worker at the same time.
and we are using 'is_parallel_safe(PlannerInfo *root, Node *node)' to see
if it is safe/necessary to generate partial path on a RelOptInfo. In the
code of 'is_parallel_safe':
/*
* We can't pass Params to workers at the moment either, so they are also
* parallel-restricted, unless they are PARAM_EXTERN Params or are
* PARAM_EXEC Params listed in safe_param_ids...
*/
else if (IsA(node, Param))
{
Param *param = (Param *) node;
if (param->paramkind == PARAM_EXTERN)
return false;
if (param->paramkind != PARAM_EXEC ||
!list_member_int(context->safe_param_ids, param->paramid))
{
if (max_parallel_hazard_test(PROPARALLEL_RESTRICTED, context))
return true;
}
return false; /* nothing to recurse to */
}
Then see the below example:
create table bigt (a int, b int, c int);
insert into bigt select i, i, i from generate_series(1, 1000000)i;
analyze bigt;
select * from bigt o where b = 1;
QUERY PLAN
-----------------------------------
Gather
Workers Planned: 2
-> Parallel Seq Scan on bigt o
Filter: (b = 1)
(4 rows)
select * from bigt o where b = 1 and c = (select sum(c) from bigt i where c = o.c);
QUERY PLAN
-------------------------------------------
Seq Scan on bigt o
Filter: ((b = 1) AND (c = (SubPlan 1)))
SubPlan 1
-> Aggregate
-> Seq Scan on bigt i
Filter: (c = o.c)
(6 rows)
I think the below plan should be correct and more efficiently.
Plan 1:
QUERY PLAN
-------------------------------------------------
Gather
Workers Planned: 2
-> Parallel Seq Scan on bigt o
Filter: ((b = 1) AND (c = (SubPlan 1)))
SubPlan 1
-> Aggregate
-> Seq Scan on bigt
Filter: (c = o.c)
(8 rows)
However the above plan is impossible because:
(1). During the planning of the SubPlan, we use is_parallel_safe() to
set the "bigt i"'s consider_parallel to false because of the above
"PARAM_EXEC" reason.
(2). The parallel_safe of the final SubPlan is set to false due to
rel->consider_parallel.
(3). During the planning of "bigt o", it calls is_parallel_safe and then
it find a subplan->parallel_safe == false, then all the partial path is
impossible.
is_parallel_safe/max_parallel_hazard_walker:
else if (IsA(node, SubPlan))
{
SubPlan *subplan = (SubPlan *) node;
List *save_safe_param_ids;
if (!subplan->parallel_safe &&
max_parallel_hazard_test(PROPARALLEL_RESTRICTED, context))
return true;
...
}
So if we think "plan 1" is valid, then what is wrong?
I think it is better to think about what parallel_safe is designed
for. In Path:
/* OK to use as part of parallel plan? */
bool parallel_safe;
The definition looks to say: the Path/Plan should not be run as a
'parallel_aware' plan, but the code looks to say: The Path/Plan should not be
run in a parallel worker even it is *not* parallel_aware. The reason I
feel the above is because:
* We don't generate parameterized partial paths for several reasons. Most
* importantly, they're not safe to execute, because there's nothing to
* make sure that a parallel scan within the parameterized portion of the
* plan is running with the same value in every worker at the same time.
If a plan which is not parallel-aware, why should we care about the
above stuff?
In the current code, there are some other parallel_safe = false which look
like a *should not be run in a parallel worker rather than parallel
plan*. the cases I know are:
1. path->parallel_safe = false in Gather/GatherMerge.
2. some expressions which is clearly claried as parallel unsafe.
So parallel_safe looks have two different meaning to me. are you feeling
something similar? Do you think treating the different parallel_safe
would make parallel works in some more places? Do you think the benefits
would be beyond the SubPlan one (I can't make a example beside SubPlan
so far).
--
Best Regards
Andy Fan
Andy Fan <zhihuifan1213@163.com> writes: Hi, Some clearer idea are provided below. Any feedback which could tell this is *obviously wrong* or *not obviously wrong* is welcome. > see the below example: > > create table bigt (a int, b int, c int); > insert into bigt select i, i, i from generate_series(1, 1000000)i; > analyze bigt; > > select * from bigt o where b = 1 and c = (select sum(c) from bigt i where c = o.c); .. > I think the below plan should be correct and more efficiently but is impossible. > > Plan 1: > > QUERY PLAN > ------------------------------------------------- > Gather > Workers Planned: 2 > -> Parallel Seq Scan on bigt o > Filter: ((b = 1) AND (c = (SubPlan 1))) > SubPlan 1 > -> Aggregate > -> Seq Scan on bigt > Filter: (c = o.c) > (8 rows) > > because: > > (1). During the planning of the SubPlan, we use is_parallel_safe() to > set the "bigt i"'s consider_parallel to false because of the above > "PARAM_EXEC" reason. > > (2). The parallel_safe of the final SubPlan is set to false due to > rel->consider_parallel. > > (3). During the planning of "bigt o", it calls is_parallel_safe and then > it find a subplan->parallel_safe == false, then all the partial path is > impossible. > > > I think it is better to think about what parallel_safe is designed > for. In Path: > > The definition looks to say: (1) the Path/Plan should not be run as a > 'parallel_aware' plan, but the code looks to say: (2). The Path/Plan > should not be run in a parallel worker even it is *not* > parallel_aware. .. > So parallel_safe looks have two different meaning to me. I'd like to revist 'bool parallel_safe' to 'ParallelSafety parallel_safe' for RelOptInfo, Path and Plan (I'd like to rename RelOptInfo->consider_parallel to parallel_safe for consistentence). ParallelSafety would contains 3 properties: 1. PARALLEL_UNSAFE = 0 // default. This acts exactly same as the current paralle_safe = false. When it is set on RelOptInfo, non partial pathlist on this RelOptInfo should be considered. When it is set to Path/Plan, no parallel worker should run the Path/Plan. 2. PARALLEL_WORKER_SAFE = 1 // We can set parallel_safe to this value for the PARAM_EXEC case (when parallel-unsafe function and Gather/MergeGather doesn't exist), The theory behind it is for a non-partial-path, it always populate a complete/same result, no matter different workers use different PARAM_EXEC values. the impact is no partial path should be considered on this RelOptInfo, but the non-partial-path/plan could be used with other partial path. 3. PARALLEL_PARTIALPATH_SAFE = 2: same as the parallel_safe=true. After this design, more Plan with SubPlan could be parallelized. Take my case for example: select * from bigt o where b = 1 and c = (select sum(c) from bigt i where c = o.c); RelOptInfo of 'bigt i' would have a parallel_safe = PARALLEL_WORKER_SAFE, so non partial path should be generated. and the final SubPlan would have a parallel_safe = PARALLEL_WORKER_SAFE. When planning RelOptInfo of 'bigt o', it only check if the SubPlan->parallel_safe is PARALLEL_UNSAFE, so at last RelOptInfo->parallel_safe is PARALLEL_PARTIALPATH_SAFE, then we could populated partial_pathlist for it. and the desired plan could be generated. -- Best Regards Andy Fan
Andy Fan <zhihuifan1213@163.com> writes: Hi, > Andy Fan <zhihuifan1213@163.com> writes: > > Hi, > > After some coding with this subject, I think it is better redefining > the problem and solution. .. > Hope I have made myself clear, any feedback is welcome! While I was registering this patch to commitfest, I found thread [1] which wanted to improve the exact same question, but the solution is different[2]. So anyone who is interested with this topic, probably want to have a check on [1] as well. [1] https://www.postgresql.org/message-id/CAAaqYe_x1u3V4uPiv%3DdJ%3Dk2EJ7txhdq6yexJDkYZ1x1pu0QwcQ%40mail.gmail.com [2] https://www.postgresql.org/message-id/87frfd9xqx.fsf%40163.com commitfest for this thread: https://commitfest.postgresql.org/patch/5892/ -- Best Regards Andy Fan