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

trexxet2 · 2023-10-30T07:14:07Z

Depending on the usage of the shared CTE with General or SegmentGeneral
subplan, the number of segments the CTE is executed on may be single for joins
with Singleton nodes, and multiple for nodes with other types of locus. In the
current implementation this may lead to deadlock if the CTE is used for both
join targets: the Join with the Singleton node results in the Share Input Scan
producer being executed with 1-gang, while the Join with the N-gang node
creates Share Input Scan reader on multiple segments, so the plan execution
hangs for the SegmentGeneral, and assertions fail inside
shareinput_mutator_xslice_*() functions for the General. If we force execution
of CTE on multiple segments, it will cause redundant motions in case of
joining the CTE with another General or SegmentGeneral. At the moment of
constructing and sharing the CTE we don't know the rest of the plan, so we
can't predict the correct CTE locus. Because replicated tables are considered
small, the most universal and optimal way to fix deadlock would be to inline
CTE scans with General or SegmentGeneral locus.

Inlining plan is implemented by making the CTE subplan once and then copying
it instead of sharing. This solution, however, cannot push filters down, because
this must be done before building the subplan, and the subplan locus can't be
known before it's construction.

src/test/regress/sql/with.sql

src/backend/optimizer/path/allpaths.c

src/test/regress/sql/with.sql

trexxet2 · 2023-10-31T06:58:37Z

the most universal and optimal way to fix deadlock would be to inline CTE scans with SegmentGeneral locus

Does this mean that 7X did not choose the most optimal way to solve the problem?

Yes, 7x makes CTE scan on replicated table on a single segment, shares it and makes Redistribute to spread it across segments:

 Hash Join
   Hash Cond: (share0_ref2.a = share0_ref1.a)
   ->  Limit
         ->  Gather Motion 3:1  (slice1; segments: 3)
               ->  Limit
                     ->  Hash Join
                           Hash Cond: (d.a = share0_ref2.a)
                           ->  Seq Scan on d
                           ->  Hash
                                 ->  Redistribute Motion 1:3  (slice2)
                                       Hash Key: share0_ref2.a
                                       ->  Shared Scan (share slice:id 2:0)
   ->  Hash
         ->  Shared Scan (share slice:id 0:0)
               ->  Aggregate
                     ->  Gather Motion 1:1  (slice3; segments: 1)
                           ->  Seq Scan on r
 Optimizer: Postgres query optimizer
(18 rows)

As I said, the problem is within the nature of Shared Scan - for Joins with Hashed and Singleton we want to perform scan on replicated table on a different subsets of segments, but using Shared Scan we can do it only over a single subset.

src/test/regress/sql/with.sql

src/backend/optimizer/path/allpaths.c

src/test/regress/sql/with.sql

src/backend/optimizer/path/allpaths.c

KnightMurloc · 2023-11-09T12:17:09Z

In the PR description, describe in more detail the proposed solution and indicate a known problem (lack of push down filters). also align the description with 80 characters and add the task number to the PR title.

KnightMurloc · 2023-11-21T05:32:44Z

add to the description that inline is implemented by single planning and copying the plan later.

Depending on the usage of the shared CTE with SegmentGeneral subplan, the number of segments the CTE are executed on may be single for joins with Singleton nodes, and multiple for hashed nodes. In the current implementation this may lead to deadlock if the CTE is used for both join targets: the Join with the Singleton node results in the Share Input Scan producer being executed on a single segment, while the Join with the Hashed node creates Share Input Scan reader on multiple segments, so the plan execution hangs. If we force execution of CTE on multiple segments as well, it will cause redundant motions in case of joining the CTE with another SegmentGeneral. At the moment of constructing and sharing the CTE we don't know the rest of the plan, so we can't predict the correct CTE locus. Because replicated tables are considered small, the most universal and optimal way to fix deadlock would be to inline CTE scans with SegmentGeneral locus. This patch fixes the deadlock by disabling sharing CTE if the subplan has SegmentGeneral locus.

src/test/regress/sql/with.sql

src/include/nodes/relation.h

src/backend/optimizer/path/allpaths.c

BenderArenadata · 2023-12-06T22:29:40Z

Failed job Regression tests with Postgres on ppc64le: https://gitlab.adsw.io/arenadata/github_mirroring/gpdb/-/jobs/860245

BenderArenadata · 2023-12-06T23:02:36Z

Failed job Regression tests with ORCA on ppc64le: https://gitlab.adsw.io/arenadata/github_mirroring/gpdb/-/jobs/860247

BenderArenadata · 2023-12-07T03:32:39Z

Failed job Resource group isolation tests on ppc64le: https://gitlab.adsw.io/arenadata/github_mirroring/gpdb/-/jobs/860692

BenderArenadata · 2023-12-07T05:18:27Z

Failed job Regression tests with Postgres on ppc64le: https://gitlab.adsw.io/arenadata/github_mirroring/gpdb/-/jobs/860688

BenderArenadata · 2023-12-07T05:29:00Z

Failed job Regression tests with Postgres on x86_64: https://gitlab.adsw.io/arenadata/github_mirroring/gpdb/-/jobs/860687

BenderArenadata · 2023-12-07T05:37:34Z

Failed job Regression tests with ORCA on ppc64le: https://gitlab.adsw.io/arenadata/github_mirroring/gpdb/-/jobs/860690

BenderArenadata · 2023-12-07T05:49:19Z

Failed job Regression tests with ORCA on x86_64: https://gitlab.adsw.io/arenadata/github_mirroring/gpdb/-/jobs/860689

BenderArenadata · 2023-12-07T10:14:18Z

Allure report https://allure-ee.adsw.io/launch/59946

BenderArenadata · 2023-12-07T10:18:59Z

Failed job Resource group isolation tests on x86_64: https://gitlab.adsw.io/arenadata/github_mirroring/gpdb/-/jobs/861409

BenderArenadata · 2023-12-07T10:31:33Z

Failed job Resource group isolation tests on ppc64le: https://gitlab.adsw.io/arenadata/github_mirroring/gpdb/-/jobs/861411

src/backend/optimizer/path/allpaths.c

BenderArenadata · 2023-12-08T11:53:23Z

Allure report https://allure-ee.adsw.io/launch/60061

BenderArenadata · 2023-12-08T12:05:05Z

Failed job Resource group isolation tests on x86_64: https://gitlab.adsw.io/arenadata/github_mirroring/gpdb/-/jobs/867698

BenderArenadata · 2023-12-08T12:10:14Z

Failed job Resource group isolation tests on ppc64le: https://gitlab.adsw.io/arenadata/github_mirroring/gpdb/-/jobs/867699

trexxet2 mentioned this pull request Oct 30, 2023

Fix potential lock by shared SegmentGeneral CTE #639

Closed

andr-sokolov requested review from KnightMurloc, andr-sokolov and bimboterminator1 October 30, 2023 13:21

This comment was marked as resolved.

Sign in to view

RekGRpth reviewed Oct 31, 2023

View reviewed changes

src/test/regress/sql/with.sql Outdated Show resolved Hide resolved

KnightMurloc reviewed Oct 31, 2023

View reviewed changes