From 2e6a363a66bd441d4634135135b771925b6b31c1 Mon Sep 17 00:00:00 2001 From: Jimmy Yih Date: Mon, 24 Jan 2022 17:05:27 -0800 Subject: [PATCH] Fix concurrent deadlock scenario with DROP INDEX on partitioned index When dropping a partitioned index, the locking order starts with the partitioned table, then its partitioned index, then the partition indexes dependent on that partitioned index, and finally the dependent partition indexes' parent tables. This order allows a deadlock scenario to happen if for example an ANALYZE happens on one of the partition tables which locks the partition table and then blocks when it attempts to lock its index (the DROP INDEX has the index lock but cannot get the partition table lock). In order to prevent this deadlock scenario, we should find and lock all the sub-partition and partition tables beneath the partitioned index's partitioned table before attempting to lock the sub-partition and partition indexes. Co-authored-by: Gaurab Dey --- src/backend/commands/tablecmds.c | 20 ++++++ .../expected/partition-drop-index-locking.out | 62 +++++++++++++++++++ src/test/isolation/isolation_schedule | 1 + .../specs/partition-drop-index-locking.spec | 47 ++++++++++++++ 4 files changed, 130 insertions(+) create mode 100644 src/test/isolation/expected/partition-drop-index-locking.out create mode 100644 src/test/isolation/specs/partition-drop-index-locking.spec diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 23364d3c7e..757888b868 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -1596,6 +1596,26 @@ RangeVarCallbackForDropRelation(const RangeVar *rel, Oid relOid, Oid oldRelOid, state->heapOid = IndexGetRelation(relOid, true); if (OidIsValid(state->heapOid)) LockRelationOid(state->heapOid, heap_lockmode); + + /* + * If we're dealing with a partitioned index, we need to find all the + * sub-partition and partition tables beneath the partitioned table + * and lock them too because DROP INDEX will lock their respective + * indexes. + */ + if (classform->relkind == RELKIND_PARTITIONED_INDEX) + { + List *inhoids; + ListCell *lc; + inhoids = find_all_inheritors(state->heapOid, NoLock, NULL); + + foreach(lc, inhoids) + { + Oid partoid = lfirst_oid(lc); + + LockRelationOid(partoid, heap_lockmode); + } + } } /* diff --git a/src/test/isolation/expected/partition-drop-index-locking.out b/src/test/isolation/expected/partition-drop-index-locking.out new file mode 100644 index 0000000000..ad42442f2a --- /dev/null +++ b/src/test/isolation/expected/partition-drop-index-locking.out @@ -0,0 +1,62 @@ +Parsed test spec with 4 sessions + +starting permutation: s1begin s2begin s1select s2drop s4getlocks s1commit s2commit +step s1begin: BEGIN; +step s2begin: BEGIN; +step s1select: SELECT * FROM part_drop_index_locking_subpart_child; +id +-- +(0 rows) + +step s2drop: DROP INDEX part_drop_index_locking_idx; +step s4getlocks: + SELECT s.query, c.relname, l.mode, l.granted + FROM pg_locks l + JOIN pg_class c ON l.relation = c.oid + JOIN pg_stat_activity s ON l.pid = s.pid + WHERE c.relname LIKE 'part_drop_index_locking%' + ORDER BY s.query, c.relname, l.mode, l.granted; + +query |relname |mode |granted +----------------------------------------------------+---------------------------------------------+-------------------+------- +DROP INDEX part_drop_index_locking_idx; |part_drop_index_locking |AccessExclusiveLock|t +DROP INDEX part_drop_index_locking_idx; |part_drop_index_locking_subpart |AccessExclusiveLock|t +DROP INDEX part_drop_index_locking_idx; |part_drop_index_locking_subpart_child |AccessExclusiveLock|f +SELECT * FROM part_drop_index_locking_subpart_child;|part_drop_index_locking_subpart_child |AccessShareLock |t +SELECT * FROM part_drop_index_locking_subpart_child;|part_drop_index_locking_subpart_child_id_idx |AccessShareLock |t +SELECT * FROM part_drop_index_locking_subpart_child;|part_drop_index_locking_subpart_child_id_idx1|AccessShareLock |t +(6 rows) + +step s1commit: COMMIT; +step s2drop: <... completed> +step s2commit: COMMIT; + +starting permutation: s1begin s3begin s1select s3drop s4getlocks s1commit s3commit +step s1begin: BEGIN; +step s3begin: BEGIN; +step s1select: SELECT * FROM part_drop_index_locking_subpart_child; +id +-- +(0 rows) + +step s3drop: DROP INDEX part_drop_index_locking_subpart_idx; +step s4getlocks: + SELECT s.query, c.relname, l.mode, l.granted + FROM pg_locks l + JOIN pg_class c ON l.relation = c.oid + JOIN pg_stat_activity s ON l.pid = s.pid + WHERE c.relname LIKE 'part_drop_index_locking%' + ORDER BY s.query, c.relname, l.mode, l.granted; + +query |relname |mode |granted +----------------------------------------------------+---------------------------------------------+-------------------+------- +DROP INDEX part_drop_index_locking_subpart_idx; |part_drop_index_locking_subpart |AccessExclusiveLock|t +DROP INDEX part_drop_index_locking_subpart_idx; |part_drop_index_locking_subpart_child |AccessExclusiveLock|f +SELECT * FROM part_drop_index_locking_subpart_child;|part_drop_index_locking_subpart_child |AccessShareLock |t +SELECT * FROM part_drop_index_locking_subpart_child;|part_drop_index_locking_subpart_child_id_idx |AccessShareLock |t +SELECT * FROM part_drop_index_locking_subpart_child;|part_drop_index_locking_subpart_child_id_idx1|AccessShareLock |t +(5 rows) + +step s1commit: COMMIT; +step s3drop: <... completed> +step s3commit: COMMIT; diff --git a/src/test/isolation/isolation_schedule b/src/test/isolation/isolation_schedule index 99c23b16ff..6a477903a6 100644 --- a/src/test/isolation/isolation_schedule +++ b/src/test/isolation/isolation_schedule @@ -89,6 +89,7 @@ test: predicate-hash test: predicate-gist test: predicate-gin test: partition-concurrent-attach +test: partition-drop-index-locking test: partition-key-update-1 test: partition-key-update-2 test: partition-key-update-3 diff --git a/src/test/isolation/specs/partition-drop-index-locking.spec b/src/test/isolation/specs/partition-drop-index-locking.spec new file mode 100644 index 0000000000..215d8e7289 --- /dev/null +++ b/src/test/isolation/specs/partition-drop-index-locking.spec @@ -0,0 +1,47 @@ +# Verify that DROP INDEX properly locks all downward sub-partitions +# and partitions before locking the indexes. + +setup +{ + CREATE TABLE part_drop_index_locking (id int) PARTITION BY RANGE(id); + CREATE TABLE part_drop_index_locking_subpart PARTITION OF part_drop_index_locking FOR VALUES FROM (1) TO (100) PARTITION BY RANGE(id); + CREATE TABLE part_drop_index_locking_subpart_child PARTITION OF part_drop_index_locking_subpart FOR VALUES FROM (1) TO (100); + CREATE INDEX part_drop_index_locking_idx ON part_drop_index_locking(id); + CREATE INDEX part_drop_index_locking_subpart_idx ON part_drop_index_locking_subpart(id); +} + +teardown +{ + DROP TABLE part_drop_index_locking; +} + +session s1 +step s1begin { BEGIN; } +step s1select { SELECT * FROM part_drop_index_locking_subpart_child; } +step s1commit { COMMIT; } + +session s2 +step s2begin { BEGIN; } +step s2drop { DROP INDEX part_drop_index_locking_idx; } +step s2commit { COMMIT; } + +session s3 +step s3begin { BEGIN; } +step s3drop { DROP INDEX part_drop_index_locking_subpart_idx; } +step s3commit { COMMIT; } + +session s4 +step s4getlocks { + SELECT s.query, c.relname, l.mode, l.granted + FROM pg_locks l + JOIN pg_class c ON l.relation = c.oid + JOIN pg_stat_activity s ON l.pid = s.pid + WHERE c.relname LIKE 'part_drop_index_locking%' + ORDER BY s.query, c.relname, l.mode, l.granted; + } + +# Run DROP INDEX on top partitioned table +permutation s1begin s2begin s1select s2drop(s1commit) s4getlocks s1commit s2commit + +# Run DROP INDEX on top sub-partition table +permutation s1begin s3begin s1select s3drop(s1commit) s4getlocks s1commit s3commit -- 2.24.3 (Apple Git-128)