0003-Relieve-RelationGetPartitionDispatchInfo-of-doing-an.patch
text/plain
Filename: 0003-Relieve-RelationGetPartitionDispatchInfo-of-doing-an.patch
Type: text/plain
Part: 2
Patch
Format: format-patch
Series: patch 0003
Subject: Relieve RelationGetPartitionDispatchInfo() of doing any locking
| File | + | − |
|---|---|---|
| src/backend/catalog/partition.c | 20 | 22 |
| src/backend/executor/execMain.c | 11 | 9 |
| src/include/catalog/partition.h | 2 | 2 |
From 6ae18ec3456b2a3fedd239059687873ae91ddbee Mon Sep 17 00:00:00 2001
From: amit <amitlangote09@gmail.com>
Date: Wed, 9 Aug 2017 15:34:45 +0900
Subject: [PATCH 3/5] Relieve RelationGetPartitionDispatchInfo() of doing any
locking
Anyone who wants to call RelationGetPartitionDispatchInfo() must first
acquire locks using find_all_inheritors.
---
src/backend/catalog/partition.c | 42 ++++++++++++++++++++---------------------
src/backend/executor/execMain.c | 20 +++++++++++---------
src/include/catalog/partition.h | 4 ++--
3 files changed, 33 insertions(+), 33 deletions(-)
diff --git a/src/backend/catalog/partition.c b/src/backend/catalog/partition.c
index 465e4fc097..4c16bf143b 100644
--- a/src/backend/catalog/partition.c
+++ b/src/backend/catalog/partition.c
@@ -1003,14 +1003,12 @@ get_partition_qual_relid(Oid relid)
* one member, that is, one for the root partitioned table), *leaf_part_oids
* contains a list of the OIDs of of all the leaf partitions.
*
- * Note that we lock only those partitions that are partitioned tables, because
- * we need to look at its relcache entry to get its PartitionKey and its
- * PartitionDesc. It's the caller's responsibility to lock the leaf partitions
- * that will actually be accessed during a given query.
+ * It is assumed that the caller has locked at least all the partitioned tables
+ * in the tree, because we need to look at their relcache entries.
*/
void
-RelationGetPartitionDispatchInfo(Relation rel, int lockmode,
- List **ptinfos, List **leaf_part_oids)
+RelationGetPartitionDispatchInfo(Relation rel, List **ptinfos,
+ List **leaf_part_oids)
{
List *all_parts,
*all_parents;
@@ -1025,16 +1023,10 @@ RelationGetPartitionDispatchInfo(Relation rel, int lockmode,
* Starting with the root partitioned table for which we already have the
* relcache entry, we look at its partition descriptor to get the
* partition OIDs. For partitions that are themselves partitioned tables,
- * we get their relcache entries after locking them with lockmode and
- * queue their partitions to be looked at later. Leaf partitions are
- * added to the result list without locking. For each partitioned table,
- * we build a PartitionedTableInfo object and add it to the other result
- * list.
- *
- * Since RelationBuildPartitionDescriptor() puts partitions in a canonical
- * order determined by comparing partition bounds, we can rely that
- * concurrent backends see the partitions in the same order, ensuring that
- * there are no deadlocks when locking the partitions.
+ * we get their relcache entries and queue their partitions to be looked at
+ * later. For each leaf partition, we simply add its OID to the result
+ * list and for each partitioned table, we build a PartitionedTableInfo
+ * object and add it to the other result list.
*/
i = offset = 0;
*ptinfos = *leaf_part_oids = NIL;
@@ -1057,8 +1049,14 @@ RelationGetPartitionDispatchInfo(Relation rel, int lockmode,
PartitionedTableInfo *ptinfo;
PartitionDispatch pd;
+ /*
+ * All the relations in the partition tree must be locked
+ * by the caller.
+ *
+ * XXX - Add RelationLockHeldByMe(partrelid) check here!
+ */
if (partrelid != RelationGetRelid(rel))
- partrel = heap_open(partrelid, lockmode);
+ partrel = heap_open(partrelid, NoLock);
else
partrel = rel;
@@ -1077,7 +1075,8 @@ RelationGetPartitionDispatchInfo(Relation rel, int lockmode,
/*
* XXX- do we need a pinning mechanism for partition descriptors
* so that there references can be managed independently of
- * the parent relcache entry? Like PinPartitionDesc(partdesc)?
+ * the fate of parent relcache entry?
+ * Like PinPartitionDesc(partdesc)?
*/
pd->partdesc = partdesc;
@@ -1141,10 +1140,9 @@ RelationGetPartitionDispatchInfo(Relation rel, int lockmode,
/*
* Release the relation descriptor. Lock that we have on the
- * table will keep the PartitionDesc that is pointing into
- * RelationData intact, a pointer to which hope to keep
- * through this transaction's commit.
- * (XXX - how true is that?)
+ * table will keep PartitionDesc (that is pointing into
+ * RelationData) intact, a reference to which want to keep through
+ * this transaction's commit. (XXX - how true is that?)
*/
if (partrel != rel)
heap_close(partrel, NoLock);
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index 0379e489d9..3dd620fc8a 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -43,6 +43,7 @@
#include "access/xact.h"
#include "catalog/namespace.h"
#include "catalog/partition.h"
+#include "catalog/pg_inherits_fn.h"
#include "catalog/pg_publication.h"
#include "commands/matview.h"
#include "commands/trigger.h"
@@ -3250,14 +3251,16 @@ ExecSetupPartitionTupleRouting(Relation rel,
int i;
ResultRelInfo *leaf_part_rri;
Relation parent;
+ List *all_parts;
/*
- * Get information about the partition tree. All the partitioned
- * tables in the tree are locked, but not the leaf partitions. We
- * lock them while building their ResultRelInfos below.
+ * Get information about the partition tree. First lock all the
+ * partitions using find_all_inheritors().
*/
- RelationGetPartitionDispatchInfo(rel, RowExclusiveLock,
- &ptinfos, &leaf_parts);
+ all_parts = find_all_inheritors(RelationGetRelid(rel), RowExclusiveLock,
+ NULL, NULL);
+ list_free(all_parts);
+ RelationGetPartitionDispatchInfo(rel, &ptinfos, &leaf_parts);
/*
* The ptinfos list contains PartitionedTableInfo objects for all the
@@ -3396,11 +3399,10 @@ ExecSetupPartitionTupleRouting(Relation rel,
TupleDesc part_tupdesc;
/*
- * RelationGetPartitionDispatchInfo didn't lock the leaf partitions,
- * so lock here. Note that each of the relations in *partitions are
- * eventually closed (when the plan is shut down, for instance).
+ * Note that each of the relations in *partitions are eventually
+ * closed (when the plan is shut down, for instance).
*/
- partrel = heap_open(lfirst_oid(cell), RowExclusiveLock);
+ partrel = heap_open(lfirst_oid(cell), NoLock); /* already locked */
part_tupdesc = RelationGetDescr(partrel);
/*
diff --git a/src/include/catalog/partition.h b/src/include/catalog/partition.h
index 6a0c81b3bd..9e63020c82 100644
--- a/src/include/catalog/partition.h
+++ b/src/include/catalog/partition.h
@@ -72,8 +72,8 @@ extern List *map_partition_varattnos(List *expr, int target_varno,
extern List *RelationGetPartitionQual(Relation rel);
extern Expr *get_partition_qual_relid(Oid relid);
-extern void RelationGetPartitionDispatchInfo(Relation rel, int lockmode,
- List **ptinfos, List **leaf_part_oids);
+extern void RelationGetPartitionDispatchInfo(Relation rel, List **ptinfos,
+ List **leaf_part_oids);
/* For tuple routing */
extern void FormPartitionKeyDatum(PartitionKeyInfo *keyinfo,
--
2.11.0