0002-Fix-lock-upgrade-deadlock-hazard-in-ATExecAttachPart.patch
text/plain
Filename: 0002-Fix-lock-upgrade-deadlock-hazard-in-ATExecAttachPart.patch
Type: text/plain
Part: 1
Patch
Format: format-patch
Series: patch 0002
Subject: Fix lock-upgrade deadlock hazard in ATExecAttachPartition
| File | + | − |
|---|---|---|
| src/backend/commands/tablecmds.c | 13 | 10 |
From 925b450cc69e8ccdbb191cb6b6d29f97aad2945f Mon Sep 17 00:00:00 2001
From: amit <amitlangote09@gmail.com>
Date: Tue, 1 Aug 2017 10:31:00 +0900
Subject: [PATCH 2/4] Fix lock-upgrade deadlock hazard in ATExecAttachPartition
Currently, we needless call find_all_inheritors twice to get the
partitions of the table being attached, with a share lock request
during the first call and exclusive lock in the second.
Fix to call find_all_inheritors() only once and request exclusive
lock on children. We need the exclusive lock, because might have
to scan the individual partitions to validate the partition
constraint being added.
---
src/backend/commands/tablecmds.c | 23 +++++++++++++----------
1 file changed, 13 insertions(+), 10 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 1307dc5893..1d879fa61a 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -13489,9 +13489,20 @@ ATExecAttachPartition(List **wqueue, Relation rel, PartitionCmd *cmd)
/*
* Prevent circularity by seeing if rel is a partition of attachRel. (In
* particular, this disallows making a rel a partition of itself.)
+ *
+ * We do that by checking if rel is a member of the list of attachRel's
+ * partitions provided the latter is partitioned at all. We want to avoid
+ * having to construct this list again, so we request the strongest lock
+ * on all partitions. We need the strongest lock, because we may decide
+ * to scan them if we find out that the table being attached (or its leaf
+ * partitions) may contain rows that violate the partition constraint.
+ * If the table has a constraint that would prevent such rows, which by
+ * definition is present in all the partitions, we need not scan the
+ * table, nor its partitions. But we cannot risk a deadlock by taking a
+ * weaker lock now and the stronger one only when needed.
*/
attachRel_children = find_all_inheritors(RelationGetRelid(attachRel),
- AccessShareLock, NULL);
+ AccessExclusiveLock, NULL);
if (list_member_oid(attachRel_children, RelationGetRelid(rel)))
ereport(ERROR,
(errcode(ERRCODE_DUPLICATE_TABLE),
@@ -13694,17 +13705,9 @@ ATExecAttachPartition(List **wqueue, Relation rel, PartitionCmd *cmd)
else
{
/* Constraints proved insufficient, so we need to scan the table. */
- List *all_parts;
ListCell *lc;
- /* Take an exclusive lock on the partitions to be checked */
- if (attachRel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
- all_parts = find_all_inheritors(RelationGetRelid(attachRel),
- AccessExclusiveLock, NULL);
- else
- all_parts = list_make1_oid(RelationGetRelid(attachRel));
-
- foreach(lc, all_parts)
+ foreach(lc, attachRel_children)
{
AlteredTableInfo *tab;
Oid part_relid = lfirst_oid(lc);
--
2.11.0