Thread: parallel_safe

parallel_safe

From
Andy Fan
Date:
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




Re: parallel_safe

From
Andy Fan
Date:
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