Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitf4b6341

Browse files
committed
Change lock acquisition order in expand_inherited_rtentry.
Previously, this function acquired locks in the order usingfind_all_inheritors(), which locks the children of each table that itprocesses in ascending OID order, and which processes the inheritancehierarchy as a whole in a breadth-first fashion. Now, it processesthe inheritance hierarchy in a depth-first fashion, and at each levelit proceeds in the order in which tables appear in the PartitionDesc.If table inheritance rather than table partitioning is used, the oldorder is preserved.This change moves the locking of any given partition much closer tothe code that actually expands that partition. This seems essentialif we ever want to allow concurrent DDL to add or remove partitions,because if the set of partitions can change, we must use the same datato decide which partitions to lock as we do to decide which partitionsto expand; otherwise, we might expand a partition that we haven'tlocked. It should hopefully also facilitate efforts to postponeinheritance expansion or locking for performance reasons, becausethere's really no way to postpone locking some partitions ifwe're blindly locking them all using find_all_inheritors().The only downside of this change which is known to me is that itfurther deviates from the principle that we should always lock theinheritance hierarchy in find_all_inheritors() order to avoid deadlockrisk. However, we've already crossed that bridge in commit9eefba1 and there are futher patchespending that make similar changes, so this isn't really giving upanything that we haven't surrendered already -- and it seems entirelyworth it, given the performance benefits some of those changes seemlikely to bring.Patch by me; thanks to David Rowley for discussion of these issues.Discussion:http://postgr.es/m/CAKJS1f_eEYVEq5tM8sm1k-HOwG0AyCPwX54XG9x4w0zy_N4Q_Q@mail.gmail.comDiscussion:http://postgr.es/m/CA+TgmoZUwPf_uanjF==gTGBMJrn8uCq52XYvAEorNkLrUdoawg@mail.gmail.com
1 parent42ccbe4 commitf4b6341

File tree

1 file changed

+28
-31
lines changed

1 file changed

+28
-31
lines changed

‎src/backend/optimizer/util/inherit.c

Lines changed: 28 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -124,28 +124,15 @@ expand_inherited_rtentry(PlannerInfo *root, RangeTblEntry *rte, Index rti)
124124

125125
/*
126126
* The rewriter should already have obtained an appropriate lock on each
127-
* relation named in the query. However, for each child relation we add
128-
* to the query, we must obtain an appropriate lock, because this will be
129-
* the first use of those relations in the parse/rewrite/plan pipeline.
130-
* Child rels should use the same lockmode as their parent.
127+
* relation named in the query, so we can open the parent relation without
128+
* locking it. However, for each child relation we add to the query, we
129+
* must obtain an appropriate lock, because this will be the first use of
130+
* those relations in the parse/rewrite/plan pipeline. Child rels should
131+
* use the same lockmode as their parent.
131132
*/
133+
oldrelation=table_open(parentOID,NoLock);
132134
lockmode=rte->rellockmode;
133135

134-
/* Scan for all members of inheritance set, acquire needed locks */
135-
inhOIDs=find_all_inheritors(parentOID,lockmode,NULL);
136-
137-
/*
138-
* Check that there's at least one descendant, else treat as no-child
139-
* case. This could happen despite above has_subclass() check, if table
140-
* once had a child but no longer does.
141-
*/
142-
if (list_length(inhOIDs)<2)
143-
{
144-
/* Clear flag before returning */
145-
rte->inh= false;
146-
return;
147-
}
148-
149136
/*
150137
* If parent relation is selected FOR UPDATE/SHARE, we need to mark its
151138
* PlanRowMark as isParent = true, and generate a new PlanRowMark for each
@@ -155,21 +142,15 @@ expand_inherited_rtentry(PlannerInfo *root, RangeTblEntry *rte, Index rti)
155142
if (oldrc)
156143
oldrc->isParent= true;
157144

158-
/*
159-
* Must open the parent relation to examine its tupdesc. We need not lock
160-
* it; we assume the rewriter already did.
161-
*/
162-
oldrelation=table_open(parentOID,NoLock);
163-
164145
/* Scan the inheritance set and expand it */
165-
if (RelationGetPartitionDesc(oldrelation)!=NULL)
146+
if (oldrelation->rd_rel->relkind==RELKIND_PARTITIONED_TABLE)
166147
{
167148
Assert(rte->relkind==RELKIND_PARTITIONED_TABLE);
168149

169150
/*
170-
* If this table has partitions, recursively expandthem in the order
171-
*in which they appear in thePartitionDesc. While at it, also
172-
*extract the partition key columns of all thepartitioned tables.
151+
* If this table has partitions, recursively expandand lock them.
152+
*While at it, also extract thepartition key columns of all the
153+
* partitioned tables.
173154
*/
174155
expand_partitioned_rtentry(root,rte,rti,oldrelation,oldrc,
175156
lockmode,&root->append_rel_list);
@@ -180,6 +161,22 @@ expand_inherited_rtentry(PlannerInfo *root, RangeTblEntry *rte, Index rti)
180161
RangeTblEntry*childrte;
181162
IndexchildRTindex;
182163

164+
/* Scan for all members of inheritance set, acquire needed locks */
165+
inhOIDs=find_all_inheritors(parentOID,lockmode,NULL);
166+
167+
/*
168+
* Check that there's at least one descendant, else treat as no-child
169+
* case. This could happen despite above has_subclass() check, if the
170+
* table once had a child but no longer does.
171+
*/
172+
if (list_length(inhOIDs)<2)
173+
{
174+
/* Clear flag before returning */
175+
rte->inh= false;
176+
heap_close(oldrelation,NoLock);
177+
return;
178+
}
179+
183180
/*
184181
* This table has no partitions. Expand any plain inheritance
185182
* children in the order the OIDs were returned by
@@ -289,8 +286,8 @@ expand_partitioned_rtentry(PlannerInfo *root, RangeTblEntry *parentrte,
289286
OidchildOID=partdesc->oids[i];
290287
Relationchildrel;
291288

292-
/* Open rel; we already have required locks */
293-
childrel=table_open(childOID,NoLock);
289+
/* Open rel, acquiring required locks */
290+
childrel=table_open(childOID,lockmode);
294291

295292
/*
296293
* Temporary partitions belonging to other sessions should have been

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp