Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ADBDEV-3929 Fix deadlock by shared SegmentGeneral CTE & planning failure by sharing General CTE #640

Merged
merged 18 commits into from
Dec 11, 2023
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 32 additions & 4 deletions src/backend/optimizer/path/allpaths.c
Original file line number Diff line number Diff line change
Expand Up @@ -2089,7 +2089,7 @@ set_cte_pathlist(PlannerInfo *root, RelOptInfo *rel, RangeTblEntry *rte)
* subplan will not be used by InitPlans, so that they can be shared
* if this CTE is referenced multiple times (excluding in InitPlans).
*/
if (cteplaninfo->shared_plan == NULL)
if (cteplaninfo->subplan == NULL)
{
PlannerConfig *config = CopyPlannerConfig(root->config);

Expand All @@ -2110,15 +2110,43 @@ set_cte_pathlist(PlannerInfo *root, RelOptInfo *rel, RangeTblEntry *rte)
subplan = subquery_planner(cteroot->glob, subquery, cteroot, cte->cterecursive,
tuple_fraction, &subroot, config);

cteplaninfo->shared_plan = prepare_plan_for_sharing(cteroot, subplan);
/*
* Sharing General and SegmentGeneral subplan may lead to deadlock
* when executed with 1-gang and joined with n-gang.
*/
if (!CdbPathLocus_IsGeneral(*subplan->flow) &&
!CdbPathLocus_IsSegmentGeneral(*subplan->flow))
{
RekGRpth marked this conversation as resolved.
Show resolved Hide resolved
cteplaninfo->subplan = prepare_plan_for_sharing(cteroot, subplan);
}
else
{
cteplaninfo->subplan = subplan;
}

cteplaninfo->subroot = subroot;
}

/*
* Create another ShareInputScan to reference the already-created
* subplan.
* subplan if not avoiding sharing for General and SegmentGeneral
RekGRpth marked this conversation as resolved.
Show resolved Hide resolved
* subplans.
*/
subplan = share_prepared_plan(cteroot, cteplaninfo->shared_plan);
if (!CdbPathLocus_IsGeneral(*cteplaninfo->subplan->flow) &&
!CdbPathLocus_IsSegmentGeneral(*cteplaninfo->subplan->flow))
{
RekGRpth marked this conversation as resolved.
Show resolved Hide resolved
subplan = share_prepared_plan(cteroot, cteplaninfo->subplan);
}
else if (subplan == NULL)
{
/*
* If we are not sharing and subplan was created just now, use it.
* Otherwise, make a copy of it to avoid construction of DAG
* instead of a tree.
*/
subplan = (Plan *) copyObject(cteplaninfo->subplan);
}

subroot = cteplaninfo->subroot;
bimboterminator1 marked this conversation as resolved.
Show resolved Hide resolved
}

Expand Down
10 changes: 6 additions & 4 deletions src/include/nodes/relation.h
Original file line number Diff line number Diff line change
Expand Up @@ -322,11 +322,13 @@ typedef struct PlannerInfo
typedef struct CtePlanInfo
{
/*
* A subplan, prepared for sharing among many CTE references by
* prepare_plan_for_sharing(), that implements the CTE. NULL if the
* CTE is not shared among references.
* A subplan, that implements the CTE and which is prepared either for
* sharing among many CTE references by prepare_plan_for_sharing() or
* for inlining in cases, when sharing produces invalid plans. NULL if
* the CTE is not shared among references (gp_cte_sharing is off), or to
* be planned or inlined and has not been planned yet.
*/
Plan *shared_plan;
Plan *subplan;

/*
* The subroot corresponding to the subplan.
Expand Down
85 changes: 85 additions & 0 deletions src/test/regress/expected/with.out
Original file line number Diff line number Diff line change
Expand Up @@ -2280,3 +2280,88 @@ WITH cte AS (

RESET optimizer;
DROP TABLE d;
-- Test if sharing is disabled for a SegmentGeneral CTE to avoid deadlock if CTE is
-- executed with 1-gang and joined with n-gang
SET optimizer = off;
--start_ignore
DROP TABLE IF EXISTS d;
NOTICE: table "d" does not exist, skipping
DROP TABLE IF EXISTS r;
NOTICE: table "r" does not exist, skipping
--end_ignore
CREATE TABLE d (a int, b int) DISTRIBUTED BY (a);
INSERT INTO d VALUES ( 1, 2 ),( 2, 3 );
CREATE TABLE r (a int, b int) DISTRIBUTED REPLICATED;
INSERT INTO r VALUES ( 1, 2 ),( 3, 4 );
EXPLAIN (COSTS off)
WITH cte AS (
SELECT count(*) a FROM r
) SELECT * FROM cte JOIN (SELECT * FROM d JOIN cte USING (a) LIMIT 1) d_join_cte USING (a);
QUERY PLAN
---------------------------------------------------------------
Hash Join
Hash Cond: (d_join_cte.a = (count(*)))
-> Subquery Scan on d_join_cte
-> Limit
-> Gather Motion 3:1 (slice1; segments: 3)
-> Limit
-> Hash Join
Hash Cond: (d.a = (count(*)))
-> Seq Scan on d
-> Hash
-> Aggregate
-> Seq Scan on r
-> Hash
-> Gather Motion 1:1 (slice2; segments: 1)
-> Aggregate
-> Seq Scan on r r_1
Optimizer: Postgres query optimizer
(17 rows)

WITH cte AS (
SELECT count(*) a FROM r
) SELECT * FROM cte JOIN (SELECT * FROM d JOIN cte USING (a) LIMIT 1) d_join_cte USING (a);
a | b
---+---
2 | 3
(1 row)

-- Test if sharing is disabled for a General CTE to avoid deadlock if CTE is
-- executed with coordinator gang and joined with n-gang
EXPLAIN (COSTS OFF)
WITH cte AS (
SELECT count(*) a FROM (VALUES ( 1, 2 ),( 3, 4 )) v
)
SELECT * FROM cte JOIN (SELECT * FROM d JOIN cte USING (a) LIMIT 1) d_join_cte USING (a);
QUERY PLAN
---------------------------------------------------------------------------
Hash Join
Hash Cond: (d_join_cte.a = (count(*)))
-> Subquery Scan on d_join_cte
-> Limit
-> Gather Motion 3:1 (slice1; segments: 3)
-> Limit
-> Hash Join
Hash Cond: (d.a = (count(*)))
-> Seq Scan on d
-> Hash
-> Aggregate
-> Values Scan on "*VALUES*"
-> Hash
-> Aggregate
-> Values Scan on "*VALUES*_1"
Optimizer: Postgres query optimizer
(16 rows)

WITH cte AS (
SELECT count(*) a FROM (VALUES ( 1, 2 ),( 3, 4 )) v
)
SELECT * FROM cte JOIN (SELECT * FROM d JOIN cte USING (a) LIMIT 1) d_join_cte USING (a);
a | b
---+---
2 | 3
(1 row)

RESET optimizer;
DROP TABLE d;
DROP TABLE r;
39 changes: 39 additions & 0 deletions src/test/regress/sql/with.sql
Original file line number Diff line number Diff line change
Expand Up @@ -1083,3 +1083,42 @@ WITH cte AS (

RESET optimizer;
DROP TABLE d;

-- Test if sharing is disabled for a SegmentGeneral CTE to avoid deadlock if CTE is
-- executed with 1-gang and joined with n-gang
SET optimizer = off;
--start_ignore
DROP TABLE IF EXISTS d;
DROP TABLE IF EXISTS r;
--end_ignore

CREATE TABLE d (a int, b int) DISTRIBUTED BY (a);
INSERT INTO d VALUES ( 1, 2 ),( 2, 3 );
CREATE TABLE r (a int, b int) DISTRIBUTED REPLICATED;
INSERT INTO r VALUES ( 1, 2 ),( 3, 4 );

EXPLAIN (COSTS off)
WITH cte AS (
SELECT count(*) a FROM r
) SELECT * FROM cte JOIN (SELECT * FROM d JOIN cte USING (a) LIMIT 1) d_join_cte USING (a);

WITH cte AS (
SELECT count(*) a FROM r
) SELECT * FROM cte JOIN (SELECT * FROM d JOIN cte USING (a) LIMIT 1) d_join_cte USING (a);
KnightMurloc marked this conversation as resolved.
Show resolved Hide resolved

-- Test if sharing is disabled for a General CTE to avoid deadlock if CTE is
-- executed with coordinator gang and joined with n-gang
EXPLAIN (COSTS OFF)
WITH cte AS (
SELECT count(*) a FROM (VALUES ( 1, 2 ),( 3, 4 )) v
)
SELECT * FROM cte JOIN (SELECT * FROM d JOIN cte USING (a) LIMIT 1) d_join_cte USING (a);

WITH cte AS (
SELECT count(*) a FROM (VALUES ( 1, 2 ),( 3, 4 )) v
)
SELECT * FROM cte JOIN (SELECT * FROM d JOIN cte USING (a) LIMIT 1) d_join_cte USING (a);

RESET optimizer;
DROP TABLE d;
DROP TABLE r;