You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudberry.apache.org by ma...@apache.org on 2025/02/07 09:28:37 UTC
(cloudberry) 23/34: Fix unlock the pruned partitions of partition table (#15665)
This is an automated email from the ASF dual-hosted git repository.
maxyang pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/cloudberry.git
commit 195b614de740e4e2ebcd334e075a98c07b86ed60
Author: Hari krishna <49...@users.noreply.github.com>
AuthorDate: Thu Jun 29 19:03:28 2023 +0530
Fix unlock the pruned partitions of partition table (#15665)
* Fix unlock the pruned partitions of partition table
FIXME's:
* GPDB_12_MERGE_FIXME: Revisit this post merge and see if we have a
chance to unlock the pruned partitions
* GPDB_12_MERGE_FIXME: Revisit this post merge and see if we have a
chance to unlock the pruned partitions
RCA:
ORCA always require root partition statistics for accurate query
planning and cost estimation. While fetching total size of a
partition table we lock all the leaf partitions including pruned
parittions, but if we find the relation statistics in MD cache,
we only acquire lock on root partition table. Although acquire
lock on every leaf partition is unnecessary, we have to acquire
lock for necessary leaf partitions on master node, otherwise it
can lead to deadlock issues.
For example :
//Assuming relation stats are available at MD Cache, so ORCA
//acquires only lock on root parittion.
txn A: select * from root_part;
txn B: truncate leaf1;
txn A and B will not block each other on QD if no locking leaf
suppose on seg0 txn A arrive first, seg1 txn B arrive first
on seg0, txn B will wait for A
on seg1, txn A will wait for B
Global Deadlock happens
Fix:
This PR fixes the unnecessary lock on leaf partitions of a
partition table and acquires the lock only on unpruned leaf
partition.
`Thumb rule is if we are going to hold a lock of table on
segment, we should first hold the lock on master (QD)`.
---
.../gpopt/translate/CTranslatorDXLToPlStmt.cpp | 12 ++++
src/backend/optimizer/util/plancat.c | 16 +++--
src/test/regress/expected/partition_locking.out | 46 ++++++++++----
...locking.out => partition_locking_optimizer.out} | 70 +++++++++++++++-------
src/test/regress/sql/partition_locking.sql | 30 ++++++----
5 files changed, 128 insertions(+), 46 deletions(-)
diff --git a/src/backend/gpopt/translate/CTranslatorDXLToPlStmt.cpp b/src/backend/gpopt/translate/CTranslatorDXLToPlStmt.cpp
index 4d7a24ee63..36e7002ad8 100644
--- a/src/backend/gpopt/translate/CTranslatorDXLToPlStmt.cpp
+++ b/src/backend/gpopt/translate/CTranslatorDXLToPlStmt.cpp
@@ -4325,10 +4325,17 @@ CTranslatorDXLToPlStmt::TranslateDXLDynTblScan(
List *oids_list = NIL;
+ const CDXLTableDescr *dxl_table_descr =
+ dyn_tbl_scan_dxlop->GetDXLTableDescr();
+ GPOS_ASSERT(dxl_table_descr->LockMode() != -1);
+
for (ULONG ul = 0; ul < parts->Size(); ul++)
{
Oid part = CMDIdGPDB::CastMdid((*parts)[ul])->Oid();
oids_list = gpdb::LAppendOid(oids_list, part);
+ // Since parser locks only root partition, locking the leaf
+ // partitions which we have to scan.
+ gpdb::GPDBLockRelationOid(part, dxl_table_descr->LockMode());
}
dyn_seq_scan->partOids = oids_list;
@@ -4409,10 +4416,15 @@ CTranslatorDXLToPlStmt::TranslateDXLDynIdxScan(
List *oids_list = NIL;
+ GPOS_ASSERT(table_desc->LockMode() != -1);
+
for (ULONG ul = 0; ul < parts->Size(); ul++)
{
Oid part = CMDIdGPDB::CastMdid((*parts)[ul])->Oid();
oids_list = gpdb::LAppendOid(oids_list, part);
+ // Since parser locks only root partition, locking the leaf
+ // partitions which we have to scan.
+ gpdb::GPDBLockRelationOid(part, table_desc->LockMode());
}
dyn_idx_scan->partOids = oids_list;
diff --git a/src/backend/optimizer/util/plancat.c b/src/backend/optimizer/util/plancat.c
index 1368bc74d5..efabcf0d22 100644
--- a/src/backend/optimizer/util/plancat.c
+++ b/src/backend/optimizer/util/plancat.c
@@ -678,9 +678,12 @@ cdb_estimate_partitioned_numtuples(Relation rel)
if (rel->rd_rel->reltuples > 0)
return rel->rd_rel->reltuples;
- inheritors = find_all_inheritors(RelationGetRelid(rel),
- AccessShareLock,
- NULL);
+ // To avoid blocking concurrent transactions on leaf partitions
+ // throughout the entire transition, we refrain from acquiring locks on
+ // the leaf partitions. Instead, we acquire locks only on the
+ // partitions that need to be scanned when ORCA writes the plan,
+ // although it may lead to less accurate stats.
+ inheritors = find_all_inheritors(RelationGetRelid(rel), NoLock, NULL);
totaltuples = 0;
foreach(lc, inheritors)
{
@@ -689,10 +692,15 @@ cdb_estimate_partitioned_numtuples(Relation rel)
double childtuples;
if (childid != RelationGetRelid(rel))
- childrel = try_table_open(childid, NoLock, false);
+ childrel = RelationIdGetRelation(childid);
else
childrel = rel;
+ // If childrel is NULL, continue by assuming the child relation
+ // has 0 tuples.
+ if (childrel == NULL)
+ continue;
+
childtuples = childrel->rd_rel->reltuples;
if (gp_enable_relsize_collection && childtuples == -1)
diff --git a/src/test/regress/expected/partition_locking.out b/src/test/regress/expected/partition_locking.out
index 1811b1f439..486e716844 100644
--- a/src/test/regress/expected/partition_locking.out
+++ b/src/test/regress/expected/partition_locking.out
@@ -7,11 +7,6 @@
-- runs out of lock space, you can work around that by simply bumping up
-- max_locks_per_transactions.
--
--- ORCA doesn't support DDL queries on partitioned tables and falls back to
--- planner. However, the locking pattern when ORCA falls back is be different
--- when ICG is run in assert vs non-assert modes. Revisit this once DML
--- queries are supported by ORCA
-set optimizer = off;
-- Show locks in master and in segments. Because the number of segments
-- in the cluster depends on configuration, we print only summary information
-- of the locks in segments. If a relation is locked only on one segment,
@@ -140,6 +135,33 @@ select * from locktest_segments where coalesce not like 'gp_%' and coalesce not
toast index | AccessExclusiveLock | relation | n segments
(28 rows)
+commit;
+-- select
+select * from partlockt where i = 1;
+ i | t
+---+---
+(0 rows)
+
+-- check locks when the relation found in MD cache
+begin;
+select * from partlockt where i = 1;
+ i | t
+---+---
+(0 rows)
+
+select * from locktest_master where coalesce not like 'gp_%' and coalesce not like 'pg_%';
+ coalesce | mode | locktype | node
+-------------------+-----------------+----------+--------
+ partlockt | AccessShareLock | relation | master
+ partlockt_1_prt_1 | AccessShareLock | relation | master
+(2 rows)
+
+select * from locktest_segments where coalesce not like 'gp_%' and coalesce not like 'pg_%';
+ coalesce | mode | locktype | node
+-------------------+-----------------+----------+-----------
+ partlockt_1_prt_1 | AccessShareLock | relation | 1 segment
+(1 row)
+
commit;
-- drop
begin;
@@ -398,7 +420,6 @@ select * from partlockt where i = 1;
---+---
(0 rows)
--- Known_opt_diff: MPP-20936
select * from locktest_master where coalesce not like 'gp_%' and coalesce not like 'pg_%';
coalesce | mode | locktype | node
-------------------------+-----------------+----------+--------
@@ -415,10 +436,16 @@ select * from locktest_segments where coalesce not like 'gp_%' and coalesce not
(2 rows)
commit;
+-- In ORCA we are seeing discrepancy with 'AccessShareLock' on root partition table
+-- with-assert and without-assert modes of DML queries, because
+-- with-assert mode we are forced to release the cache, which leads to acquire
+-- 'AccessShareLock' always on master. Since the AccessShareLock on root
+-- partition is redundant so we are skipping to evaluate.
begin;
-- insert locking
insert into partlockt values(3, 'f');
-select * from locktest_master where coalesce not like 'gp_%' and coalesce not like 'pg_%';
+select * from locktest_master where coalesce not like 'gp_%' and coalesce not like 'pg_%' and
+ not(coalesce like 'partlockt' and mode like 'AccessShareLock');
coalesce | mode | locktype | node
-----------+------------------+----------+--------
partlockt | RowExclusiveLock | relation | master
@@ -435,8 +462,8 @@ commit;
-- delete locking
begin;
delete from partlockt where i = 4;
--- Known_opt_diff: MPP-20936
-select * from locktest_master where coalesce not like 'gp_%' and coalesce not like 'pg_%';
+select * from locktest_master where coalesce not like 'gp_%' and coalesce not like 'pg_%' and
+ not(coalesce like 'partlockt' and mode like 'AccessShareLock');
coalesce | mode | locktype | node
-------------------------+---------------+----------+--------
partlockt | ExclusiveLock | relation | master
@@ -543,4 +570,3 @@ select * from locktest_segments where coalesce not like 'gp_%' and coalesce not
(38 rows)
commit;
-reset optimizer;
diff --git a/src/test/regress/expected/partition_locking.out b/src/test/regress/expected/partition_locking_optimizer.out
similarity index 93%
copy from src/test/regress/expected/partition_locking.out
copy to src/test/regress/expected/partition_locking_optimizer.out
index 1811b1f439..20b05c86db 100644
--- a/src/test/regress/expected/partition_locking.out
+++ b/src/test/regress/expected/partition_locking_optimizer.out
@@ -7,11 +7,6 @@
-- runs out of lock space, you can work around that by simply bumping up
-- max_locks_per_transactions.
--
--- ORCA doesn't support DDL queries on partitioned tables and falls back to
--- planner. However, the locking pattern when ORCA falls back is be different
--- when ICG is run in assert vs non-assert modes. Revisit this once DML
--- queries are supported by ORCA
-set optimizer = off;
-- Show locks in master and in segments. Because the number of segments
-- in the cluster depends on configuration, we print only summary information
-- of the locks in segments. If a relation is locked only on one segment,
@@ -72,7 +67,7 @@ begin;
-- creation
create table partlockt (i int, t text) partition by range(i)
(start(1) end(10) every(1));
-NOTICE: Table doesn't have 'DISTRIBUTED BY' clause -- Using column named 'i' as the Cloudberry Database data distribution key for this table.
+NOTICE: Table doesn't have 'DISTRIBUTED BY' clause -- Using column named 'i' as the Greenplum Database data distribution key for this table.
HINT: The 'DISTRIBUTED BY' clause determines the distribution of data. Make sure column(s) chosen are the optimal data distribution key to minimize skew.
select * from locktest_master where coalesce not like 'gp_%' and coalesce not like 'pg_%';
coalesce | mode | locktype | node
@@ -140,6 +135,34 @@ select * from locktest_segments where coalesce not like 'gp_%' and coalesce not
toast index | AccessExclusiveLock | relation | n segments
(28 rows)
+commit;
+-- select
+select * from partlockt where i = 1;
+ i | t
+---+---
+(0 rows)
+
+-- check locks when the relation found in MD cache
+begin;
+select * from partlockt where i = 1;
+ i | t
+---+---
+(0 rows)
+
+select * from locktest_master where coalesce not like 'gp_%' and coalesce not like 'pg_%';
+ coalesce | mode | locktype | node
+-------------------+-----------------+----------+--------
+ partlockt | AccessShareLock | relation | master
+ partlockt_1_prt_1 | AccessShareLock | relation | master
+(2 rows)
+
+select * from locktest_segments where coalesce not like 'gp_%' and coalesce not like 'pg_%';
+ coalesce | mode | locktype | node
+-------------------+-----------------+----------+-----------
+ partlockt | AccessShareLock | relation | 1 segment
+ partlockt_1_prt_1 | AccessShareLock | relation | 1 segment
+(2 rows)
+
commit;
-- drop
begin;
@@ -218,7 +241,7 @@ create table partlockt (i int, t text, n numeric)
with (appendonly = true)
partition by list(i)
(values(1), values(2), values(3));
-NOTICE: Table doesn't have 'DISTRIBUTED BY' clause -- Using column named 'i' as the Cloudberry Database data distribution key for this table.
+NOTICE: Table doesn't have 'DISTRIBUTED BY' clause -- Using column named 'i' as the Greenplum Database data distribution key for this table.
HINT: The 'DISTRIBUTED BY' clause determines the distribution of data. Make sure column(s) chosen are the optimal data distribution key to minimize skew.
select * from locktest_master where coalesce not like 'gp_%' and coalesce not like 'pg_%';
coalesce | mode | locktype | node
@@ -263,8 +286,9 @@ insert into partlockt values(1), (2), (3);
select * from locktest_master where coalesce not like 'gp_%' and coalesce not like 'pg_%';
coalesce | mode | locktype | node
-----------+------------------+----------+--------
+ partlockt | AccessShareLock | relation | master
partlockt | RowExclusiveLock | relation | master
-(1 row)
+(2 rows)
select * from locktest_segments where coalesce not like 'gp_%' and coalesce not like 'pg_%';
coalesce | mode | locktype | node
@@ -331,7 +355,7 @@ commit;
-- Indexing
create table partlockt (i int, t text) partition by range(i)
(start(1) end(10) every(1));
-NOTICE: Table doesn't have 'DISTRIBUTED BY' clause -- Using column named 'i' as the Cloudberry Database data distribution key for this table.
+NOTICE: Table doesn't have 'DISTRIBUTED BY' clause -- Using column named 'i' as the Greenplum Database data distribution key for this table.
HINT: The 'DISTRIBUTED BY' clause determines the distribution of data. Make sure column(s) chosen are the optimal data distribution key to minimize skew.
begin;
create index partlockt_idx on partlockt(i);
@@ -398,27 +422,32 @@ select * from partlockt where i = 1;
---+---
(0 rows)
--- Known_opt_diff: MPP-20936
select * from locktest_master where coalesce not like 'gp_%' and coalesce not like 'pg_%';
- coalesce | mode | locktype | node
--------------------------+-----------------+----------+--------
- partlockt | AccessShareLock | relation | master
- partlockt_1_prt_1 | AccessShareLock | relation | master
- partlockt_1_prt_1_i_idx | AccessShareLock | relation | master
-(3 rows)
+ coalesce | mode | locktype | node
+-------------------+-----------------+----------+--------
+ partlockt | AccessShareLock | relation | master
+ partlockt_1_prt_1 | AccessShareLock | relation | master
+(2 rows)
select * from locktest_segments where coalesce not like 'gp_%' and coalesce not like 'pg_%';
coalesce | mode | locktype | node
-------------------------+-----------------+----------+-----------
+ partlockt | AccessShareLock | relation | 1 segment
partlockt_1_prt_1 | AccessShareLock | relation | 1 segment
partlockt_1_prt_1_i_idx | AccessShareLock | relation | 1 segment
-(2 rows)
+(3 rows)
commit;
+-- In ORCA we are seeing discrepancy with 'AccessShareLock' on root partition table
+-- with-assert and without-assert modes of DML queries, because
+-- with-assert mode we are forced to release the cache, which leads to acquire
+-- 'AccessShareLock' always on master. Since the AccessShareLock on root
+-- partition is redundant so we are skipping to evaluate.
begin;
-- insert locking
insert into partlockt values(3, 'f');
-select * from locktest_master where coalesce not like 'gp_%' and coalesce not like 'pg_%';
+select * from locktest_master where coalesce not like 'gp_%' and coalesce not like 'pg_%' and
+ not(coalesce like 'partlockt' and mode like 'AccessShareLock');
coalesce | mode | locktype | node
-----------+------------------+----------+--------
partlockt | RowExclusiveLock | relation | master
@@ -435,8 +464,8 @@ commit;
-- delete locking
begin;
delete from partlockt where i = 4;
--- Known_opt_diff: MPP-20936
-select * from locktest_master where coalesce not like 'gp_%' and coalesce not like 'pg_%';
+select * from locktest_master where coalesce not like 'gp_%' and coalesce not like 'pg_%' and
+ not(coalesce like 'partlockt' and mode like 'AccessShareLock');
coalesce | mode | locktype | node
-------------------------+---------------+----------+--------
partlockt | ExclusiveLock | relation | master
@@ -543,4 +572,3 @@ select * from locktest_segments where coalesce not like 'gp_%' and coalesce not
(38 rows)
commit;
-reset optimizer;
diff --git a/src/test/regress/sql/partition_locking.sql b/src/test/regress/sql/partition_locking.sql
index c981f7eaa8..5d7458410d 100644
--- a/src/test/regress/sql/partition_locking.sql
+++ b/src/test/regress/sql/partition_locking.sql
@@ -8,12 +8,6 @@
-- max_locks_per_transactions.
--
--- ORCA doesn't support DDL queries on partitioned tables and falls back to
--- planner. However, the locking pattern when ORCA falls back is be different
--- when ICG is run in assert vs non-assert modes. Revisit this once DML
--- queries are supported by ORCA
-set optimizer = off;
-
-- Show locks in master and in segments. Because the number of segments
-- in the cluster depends on configuration, we print only summary information
-- of the locks in segments. If a relation is locked only on one segment,
@@ -83,6 +77,15 @@ select * from locktest_master where coalesce not like 'gp_%' and coalesce not li
select * from locktest_segments where coalesce not like 'gp_%' and coalesce not like 'pg_%';
commit;
+-- select
+select * from partlockt where i = 1;
+-- check locks when the relation found in MD cache
+begin;
+select * from partlockt where i = 1;
+select * from locktest_master where coalesce not like 'gp_%' and coalesce not like 'pg_%';
+select * from locktest_segments where coalesce not like 'gp_%' and coalesce not like 'pg_%';
+commit;
+
-- drop
begin;
drop table partlockt;
@@ -140,23 +143,29 @@ set enable_seqscan=off;
-- test select locking
begin;
select * from partlockt where i = 1;
--- Known_opt_diff: MPP-20936
select * from locktest_master where coalesce not like 'gp_%' and coalesce not like 'pg_%';
select * from locktest_segments where coalesce not like 'gp_%' and coalesce not like 'pg_%';
commit;
+-- In ORCA we are seeing discrepancy with 'AccessShareLock' on root partition table
+-- with-assert and without-assert modes of DML queries, because
+-- with-assert mode we are forced to release the cache, which leads to acquire
+-- 'AccessShareLock' always on master. Since the AccessShareLock on root
+-- partition is redundant so we are skipping to evaluate.
+
begin;
-- insert locking
insert into partlockt values(3, 'f');
-select * from locktest_master where coalesce not like 'gp_%' and coalesce not like 'pg_%';
+select * from locktest_master where coalesce not like 'gp_%' and coalesce not like 'pg_%' and
+ not(coalesce like 'partlockt' and mode like 'AccessShareLock');
select * from locktest_segments where coalesce not like 'gp_%' and coalesce not like 'pg_%';
commit;
-- delete locking
begin;
delete from partlockt where i = 4;
--- Known_opt_diff: MPP-20936
-select * from locktest_master where coalesce not like 'gp_%' and coalesce not like 'pg_%';
+select * from locktest_master where coalesce not like 'gp_%' and coalesce not like 'pg_%' and
+ not(coalesce like 'partlockt' and mode like 'AccessShareLock');
select * from locktest_segments where coalesce not like 'gp_%' and coalesce not like 'pg_%';
commit;
@@ -167,4 +176,3 @@ select * from locktest_master where coalesce not like 'gp_%' and coalesce not li
select * from locktest_segments where coalesce not like 'gp_%' and coalesce not like 'pg_%';
commit;
-reset optimizer;
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cloudberry.apache.org
For additional commands, e-mail: commits-help@cloudberry.apache.org