From 6ae18ec3456b2a3fedd239059687873ae91ddbee Mon Sep 17 00:00:00 2001 From: amit 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