Change rewriter/planner/executor/plancache to depend on RTE rellockmode.

Instead of recomputing the required lock levels in all these places,
just use what commit fdba460a2 made the parser store in the RTE fields.
This already simplifies the code measurably in these places, and
follow-on changes will remove a bunch of no-longer-needed infrastructure.

In a few cases, this change causes us to acquire a higher lock level
than we did before.  This is OK primarily because said higher lock level
should've been acquired already at query parse time; thus, we're saving
a useless extra trip through the shared lock manager to acquire a lesser
lock alongside the original lock.  The only known exception to this is
that re-execution of a previously planned SELECT FOR UPDATE/SHARE query,
for a table that uses ROW_MARK_REFERENCE or ROW_MARK_COPY methods, might
have gotten only AccessShareLock before.  Now it will get RowShareLock
like the first execution did, which seems fine.

While there's more to do, push it in this state anyway, to let the
buildfarm help verify that nothing bad happened.

Amit Langote, reviewed by David Rowley and Jesper Pedersen,
and whacked around a bit more by me

Discussion: https://postgr.es/m/468c85d9-540e-66a2-1dde-fec2b741e688@lab.ntt.co.jp
This commit is contained in:
Tom Lane 2018-10-02 14:43:01 -04:00
parent cc2905e963
commit 6e35939feb
5 changed files with 27 additions and 115 deletions

View File

@ -970,26 +970,16 @@ InitPlan(QueryDesc *queryDesc, int eflags)
/* get relation's OID (will produce InvalidOid if subquery) */
relid = getrelid(rc->rti, rangeTable);
rellockmode = rt_fetch(rc->rti, rangeTable)->rellockmode;
/*
* If you change the conditions under which rel locks are acquired
* here, be sure to adjust ExecOpenScanRelation to match.
*/
switch (rc->markType)
{
case ROW_MARK_EXCLUSIVE:
case ROW_MARK_NOKEYEXCLUSIVE:
case ROW_MARK_SHARE:
case ROW_MARK_KEYSHARE:
Assert(rellockmode == RowShareLock);
relation = heap_open(relid, RowShareLock);
break;
case ROW_MARK_REFERENCE:
/* RTE might be a query target table */
Assert(rellockmode == AccessShareLock ||
rellockmode == RowExclusiveLock);
relation = heap_open(relid, AccessShareLock);
rellockmode = rt_fetch(rc->rti, rangeTable)->rellockmode;
relation = heap_open(relid, rellockmode);
break;
case ROW_MARK_COPY:
/* no physical table access is required */

View File

@ -641,10 +641,6 @@ ExecRelationIsTargetRelation(EState *estate, Index scanrelid)
*
* Open the heap relation to be scanned by a base-level scan plan node.
* This should be called during the node's ExecInit routine.
*
* By default, this acquires AccessShareLock on the relation. However,
* if the relation was already locked by InitPlan, we don't need to acquire
* any additional lock. This saves trips to the shared lock manager.
* ----------------------------------------------------------------
*/
Relation
@ -654,37 +650,9 @@ ExecOpenScanRelation(EState *estate, Index scanrelid, int eflags)
Oid reloid;
LOCKMODE lockmode;
/*
* Determine the lock type we need. First, scan to see if target relation
* is a result relation. If not, check if it's a FOR UPDATE/FOR SHARE
* relation.
*
* Note: we may have already gotten the desired lock type, but for now
* don't try to optimize; this logic is going away soon anyhow.
*/
lockmode = AccessShareLock;
if (ExecRelationIsTargetRelation(estate, scanrelid))
lockmode = RowExclusiveLock;
else
{
/* Keep this check in sync with InitPlan! */
ExecRowMark *erm = ExecFindRowMark(estate, scanrelid, true);
if (erm != NULL)
{
if (erm->markType == ROW_MARK_REFERENCE ||
erm->markType == ROW_MARK_COPY)
lockmode = AccessShareLock;
else
lockmode = RowShareLock;
}
}
/* lockmode per above logic must not be more than we previously acquired */
Assert(lockmode <= rt_fetch(scanrelid, estate->es_range_table)->rellockmode);
/* Open the relation and acquire lock as needed */
reloid = getrelid(scanrelid, estate->es_range_table);
lockmode = rt_fetch(scanrelid, estate->es_range_table)->rellockmode;
rel = heap_open(reloid, lockmode);
/*
@ -912,6 +880,7 @@ ExecLockNonLeafAppendTables(List *partitioned_rels, EState *estate)
if (!is_result_rel)
{
PlanRowMark *rc = NULL;
LOCKMODE lockmode;
foreach(l, stmt->rowMarks)
{
@ -923,9 +892,13 @@ ExecLockNonLeafAppendTables(List *partitioned_rels, EState *estate)
}
if (rc && RowMarkRequiresRowShareLock(rc->markType))
LockRelationOid(relid, RowShareLock);
lockmode = RowShareLock;
else
LockRelationOid(relid, AccessShareLock);
lockmode = AccessShareLock;
Assert(lockmode == rt_fetch(rti, estate->es_range_table)->rellockmode);
LockRelationOid(relid, lockmode);
}
}
}

View File

@ -1515,7 +1515,6 @@ expand_inherited_tables(PlannerInfo *root)
static void
expand_inherited_rtentry(PlannerInfo *root, RangeTblEntry *rte, Index rti)
{
Query *parse = root->parse;
Oid parentOID;
PlanRowMark *oldrc;
Relation oldrelation;
@ -1546,21 +1545,9 @@ expand_inherited_rtentry(PlannerInfo *root, RangeTblEntry *rte, Index rti)
* relation named in the query. However, for each child relation we add
* to the query, we must obtain an appropriate lock, because this will be
* the first use of those relations in the parse/rewrite/plan pipeline.
*
* If the parent relation is the query's result relation, then we need
* RowExclusiveLock. Otherwise, if it's accessed FOR UPDATE/SHARE, we
* need RowShareLock; otherwise AccessShareLock. We can't just grab
* AccessShareLock because then the executor would be trying to upgrade
* the lock, leading to possible deadlocks. (This code should match the
* parser and rewriter.)
* Child rels should use the same lockmode as their parent.
*/
oldrc = get_plan_rowmark(root->rowMarks, rti);
if (rti == parse->resultRelation)
lockmode = RowExclusiveLock;
else if (oldrc && RowMarkRequiresRowShareLock(oldrc->markType))
lockmode = RowShareLock;
else
lockmode = AccessShareLock;
lockmode = rte->rellockmode;
/* Scan for all members of inheritance set, acquire needed locks */
inhOIDs = find_all_inheritors(parentOID, lockmode, NULL);
@ -1582,6 +1569,7 @@ expand_inherited_rtentry(PlannerInfo *root, RangeTblEntry *rte, Index rti)
* PlanRowMark as isParent = true, and generate a new PlanRowMark for each
* child.
*/
oldrc = get_plan_rowmark(root->rowMarks, rti);
if (oldrc)
oldrc->isParent = true;

View File

@ -87,17 +87,14 @@ static Bitmapset *adjust_view_column_set(Bitmapset *cols, List *targetlist);
* AcquireRewriteLocks -
* Acquire suitable locks on all the relations mentioned in the Query.
* These locks will ensure that the relation schemas don't change under us
* while we are rewriting and planning the query.
* while we are rewriting, planning, and executing the query.
*
* Caution: this may modify the querytree, therefore caller should usually
* have done a copyObject() to make a writable copy of the querytree in the
* current memory context.
*
* forExecute indicates that the query is about to be executed.
* If so, we'll acquire RowExclusiveLock on the query's resultRelation,
* RowShareLock on any relation accessed FOR [KEY] UPDATE/SHARE, and
* AccessShareLock on all other relations mentioned.
*
* forExecute indicates that the query is about to be executed. If so,
* we'll acquire the lock modes specified in the RTE rellockmode fields.
* If forExecute is false, AccessShareLock is acquired on all relations.
* This case is suitable for ruleutils.c, for example, where we only need
* schema stability and we don't intend to actually modify any relations.
@ -162,31 +159,24 @@ AcquireRewriteLocks(Query *parsetree,
/*
* Grab the appropriate lock type for the relation, and do not
* release it until end of transaction. This protects the
* rewriter and planner against schema changes mid-query.
* release it until end of transaction. This protects the
* rewriter, planner, and executor against schema changes
* mid-query.
*
* Assuming forExecute is true, this logic must match what the
* executor will do, else we risk lock-upgrade deadlocks.
* If forExecute is false, ignore rellockmode and just use
* AccessShareLock.
*/
if (!forExecute)
lockmode = AccessShareLock;
else if (rt_index == parsetree->resultRelation)
lockmode = RowExclusiveLock;
else if (forUpdatePushedDown)
{
lockmode = RowShareLock;
/* Upgrade RTE's lock mode to reflect pushed-down lock */
if (rte->rellockmode == AccessShareLock)
rte->rellockmode = RowShareLock;
lockmode = rte->rellockmode;
}
else if (get_parse_rowmark(parsetree, rt_index) != NULL)
lockmode = RowShareLock;
else
lockmode = AccessShareLock;
Assert(!forExecute || lockmode == rte->rellockmode ||
(lockmode == AccessShareLock &&
rte->rellockmode == RowExclusiveLock));
lockmode = rte->rellockmode;
rel = heap_open(rte->relid, lockmode);

View File

@ -1493,7 +1493,6 @@ AcquireExecutorLocks(List *stmt_list, bool acquire)
foreach(lc1, stmt_list)
{
PlannedStmt *plannedstmt = lfirst_node(PlannedStmt, lc1);
int rt_index;
ListCell *lc2;
if (plannedstmt->commandType == CMD_UTILITY)
@ -1512,14 +1511,9 @@ AcquireExecutorLocks(List *stmt_list, bool acquire)
continue;
}
rt_index = 0;
foreach(lc2, plannedstmt->rtable)
{
RangeTblEntry *rte = (RangeTblEntry *) lfirst(lc2);
LOCKMODE lockmode;
PlanRowMark *rc;
rt_index++;
if (rte->rtekind != RTE_RELATION)
continue;
@ -1530,21 +1524,10 @@ AcquireExecutorLocks(List *stmt_list, bool acquire)
* fail if it's been dropped entirely --- we'll just transiently
* acquire a non-conflicting lock.
*/
if (list_member_int(plannedstmt->resultRelations, rt_index) ||
list_member_int(plannedstmt->nonleafResultRelations, rt_index))
lockmode = RowExclusiveLock;
else if ((rc = get_plan_rowmark(plannedstmt->rowMarks, rt_index)) != NULL &&
RowMarkRequiresRowShareLock(rc->markType))
lockmode = RowShareLock;
else
lockmode = AccessShareLock;
Assert(lockmode == rte->rellockmode);
if (acquire)
LockRelationOid(rte->relid, lockmode);
LockRelationOid(rte->relid, rte->rellockmode);
else
UnlockRelationOid(rte->relid, lockmode);
UnlockRelationOid(rte->relid, rte->rellockmode);
}
}
}
@ -1586,7 +1569,6 @@ static void
ScanQueryForLocks(Query *parsetree, bool acquire)
{
ListCell *lc;
int rt_index;
/* Shouldn't get called on utility commands */
Assert(parsetree->commandType != CMD_UTILITY);
@ -1594,29 +1576,18 @@ ScanQueryForLocks(Query *parsetree, bool acquire)
/*
* First, process RTEs of the current query level.
*/
rt_index = 0;
foreach(lc, parsetree->rtable)
{
RangeTblEntry *rte = (RangeTblEntry *) lfirst(lc);
LOCKMODE lockmode;
rt_index++;
switch (rte->rtekind)
{
case RTE_RELATION:
/* Acquire or release the appropriate type of lock */
if (rt_index == parsetree->resultRelation)
lockmode = RowExclusiveLock;
else if (get_parse_rowmark(parsetree, rt_index) != NULL)
lockmode = RowShareLock;
else
lockmode = AccessShareLock;
Assert(lockmode == rte->rellockmode ||
(lockmode == AccessShareLock && rte->rellockmode == RowExclusiveLock));
if (acquire)
LockRelationOid(rte->relid, lockmode);
LockRelationOid(rte->relid, rte->rellockmode);
else
UnlockRelationOid(rte->relid, lockmode);
UnlockRelationOid(rte->relid, rte->rellockmode);
break;
case RTE_SUBQUERY: