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

Commit34f6c60

Browse files
author
Amit Kapila
committed
Revert the commits related to allowing page lock to conflict among parallel group members.
This commit reverts the work done by commits3ba59cc and72e78d8.Those commits were incorrect in asserting that we never acquire any otherheavy-weight lock after acquring page lock other than relation extensionlock. We can acquire a lock on catalogs while doing catalog look up afteracquring page lock.This won't impact any existing feature but we need to think some other wayto achieve this before parallelizing other write operations or evenimproving the parallelism in vacuum (like allowing multiple workersfor an index).Reported-by: Jaime CasanovaAuthor: Amit KapilaBackpatch-through: 13Discussion:https://postgr.es/m/CAJKUy5jffnRKNvRHKQ0LynRb0RJC-o4P8Ku3x9vGAVLwDBWumQ@mail.gmail.com
1 parent59c2a6f commit34f6c60

File tree

5 files changed

+34
-65
lines changed

5 files changed

+34
-65
lines changed

‎src/backend/optimizer/plan/planner.c

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -324,11 +324,13 @@ standard_planner(Query *parse, const char *query_string, int cursorOptions,
324324
* functions are present in the query tree.
325325
*
326326
* (Note that we do allow CREATE TABLE AS, SELECT INTO, and CREATE
327-
* MATERIALIZED VIEW to use parallel plans, but as of now, only the leader
328-
* backend writes into a completely new table. In the future, we can
329-
* extend it to allow workers to write into the table. However, to allow
330-
* parallel updates and deletes, we have to solve other problems,
331-
* especially around combo CIDs.)
327+
* MATERIALIZED VIEW to use parallel plans, but this is safe only because
328+
* the command is writing into a completely new table which workers won't
329+
* be able to see. If the workers could see the table, the fact that
330+
* group locking would cause them to ignore the leader's heavyweight
331+
* GIN page locks would make this unsafe. We'll have to fix that somehow
332+
* if we want to allow parallel inserts in general; updates and deletes
333+
* have additional problems especially around combo CIDs.)
332334
*
333335
* For now, we don't try to use parallel mode if we're running inside a
334336
* parallel worker. We might eventually be able to relax this

‎src/backend/storage/lmgr/README

Lines changed: 13 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -597,10 +597,10 @@ deadlock detection algorithm very much, but it makes the bookkeeping more
597597
complicated.
598598

599599
We choose to regard locks held by processes in the same parallel group as
600-
non-conflicting with the exception of relation extensionand page locks. This
601-
means thattwo processes in a parallel group can hold a self-exclusive lock on
602-
the samerelation at the same time, or one process can acquire an AccessShareLock
603-
whilethe other already holds AccessExclusiveLock. This might seem dangerous and
600+
non-conflicting with the exception of relation extensionlock. This means that
601+
two processes in a parallel group can hold a self-exclusive lock on the same
602+
relation at the same time, or one process can acquire an AccessShareLock while
603+
the other already holds AccessExclusiveLock. This might seem dangerous and
604604
could be in some cases (more on that below), but if we didn't do this then
605605
parallel query would be extremely prone to self-deadlock. For example, a
606606
parallel query against a relation on which the leader already had
@@ -638,23 +638,15 @@ the other is safe enough. Problems would occur if the leader initiated
638638
parallelism from a point in the code at which it had some backend-private
639639
state that made table access from another process unsafe, for example after
640640
calling SetReindexProcessing and before calling ResetReindexProcessing,
641-
catastrophe could ensue, because the worker won't have that state.
642-
643-
To allow parallel inserts and parallel copy, we have ensured that relation
644-
extension and page locks don't participate in group locking which means such
645-
locks can conflict among the same group members. This is required as it is no
646-
safer for two related processes to extend the same relation or perform clean up
647-
in gin indexes at a time than for unrelated processes to do the same. We don't
648-
acquire a heavyweight lock on any other object after relation extension lock
649-
which means such a lock can never participate in the deadlock cycle. After
650-
acquiring page locks, we can acquire relation extension lock but reverse never
651-
happens, so those will also not participate in deadlock. To allow for other
652-
parallel writes like parallel update or parallel delete, we'll either need to
653-
(1) further enhance the deadlock detector to handle those tuple locks in a
654-
different way than other types; or (2) have parallel workers use some other
655-
mutual exclusion method for such cases. Currently, the parallel mode is
656-
strictly read-only, but now we have the infrastructure to allow parallel
657-
inserts and parallel copy.
641+
catastrophe could ensue, because the worker won't have that state. Similarly,
642+
problems could occur with certain kinds of non-relation locks, such as
643+
GIN page locks. It's no safer for two related processes to perform GIN clean
644+
up at the same time than for unrelated processes to do the same.
645+
However, since parallel mode is strictly read-only at present, neither this
646+
nor most of the similar cases can arise at present. To allow parallel writes,
647+
we'll either need to (1) further enhance the deadlock detector to handle those
648+
types of locks in a different way than other types; or (2) have parallel
649+
workers use some other mutual exclusion method for such cases.
658650

659651
Group locking adds three new members to each PGPROC: lockGroupLeader,
660652
lockGroupMembers, and lockGroupLink. A PGPROC's lockGroupLeader is NULL for

‎src/backend/storage/lmgr/deadlock.c

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -556,12 +556,11 @@ FindLockCycleRecurseMember(PGPROC *checkProc,
556556
lm;
557557

558558
/*
559-
* The relation extensionor pagelock can never participate in actual
560-
*deadlockcycle. SeeAsserts in LockAcquireExtended. So, there is no
561-
*advantagein checking wait edges fromthem.
559+
* The relation extension lock can never participate in actual deadlock
560+
* cycle. SeeAssert in LockAcquireExtended. So, there is no advantage
561+
* in checking wait edges fromit.
562562
*/
563-
if (LOCK_LOCKTAG(*lock)==LOCKTAG_RELATION_EXTEND||
564-
(LOCK_LOCKTAG(*lock)==LOCKTAG_PAGE))
563+
if (LOCK_LOCKTAG(*lock)==LOCKTAG_RELATION_EXTEND)
565564
return false;
566565

567566
lockMethodTable=GetLocksMethodTable(lock);

‎src/backend/storage/lmgr/lock.c

Lines changed: 4 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -185,18 +185,6 @@ static intFastPathLocalUseCount = 0;
185185
*/
186186
staticboolIsRelationExtensionLockHeldPG_USED_FOR_ASSERTS_ONLY= false;
187187

188-
/*
189-
* Flag to indicate if the page lock is held by this backend. We don't
190-
* acquire any other heavyweight lock while holding the page lock except for
191-
* relation extension. However, these locks are never taken in reverse order
192-
* which implies that page locks will also never participate in the deadlock
193-
* cycle.
194-
*
195-
* Similar to relation extension, page locks are also held for a short
196-
* duration, so imposing such a restriction won't hurt.
197-
*/
198-
staticboolIsPageLockHeldPG_USED_FOR_ASSERTS_ONLY= false;
199-
200188
/* Macros for manipulating proc->fpLockBits */
201189
#defineFAST_PATH_BITS_PER_SLOT3
202190
#defineFAST_PATH_LOCKNUMBER_OFFSET1
@@ -886,13 +874,6 @@ LockAcquireExtended(const LOCKTAG *locktag,
886874
*/
887875
Assert(!IsRelationExtensionLockHeld);
888876

889-
/*
890-
* We don't acquire any other heavyweight lock while holding the page lock
891-
* except for relation extension.
892-
*/
893-
Assert(!IsPageLockHeld||
894-
(locktag->locktag_type==LOCKTAG_RELATION_EXTEND));
895-
896877
/*
897878
* Prepare to emit a WAL record if acquisition of this lock needs to be
898879
* replayed in a standby server.
@@ -1341,10 +1322,10 @@ SetupLockInTable(LockMethod lockMethodTable, PGPROC *proc,
13411322
}
13421323

13431324
/*
1344-
* Check and set/reset the flag that we hold the relation extension/page lock.
1325+
* Check and set/reset the flag that we hold the relation extension lock.
13451326
*
13461327
* It is callers responsibility that this function is called after
1347-
* acquiring/releasing the relation extension/page lock.
1328+
* acquiring/releasing the relation extension lock.
13481329
*
13491330
* Pass acquired as true if lock is acquired, false otherwise.
13501331
*/
@@ -1354,9 +1335,6 @@ CheckAndSetLockHeld(LOCALLOCK *locallock, bool acquired)
13541335
#ifdefUSE_ASSERT_CHECKING
13551336
if (LOCALLOCK_LOCKTAG(*locallock)==LOCKTAG_RELATION_EXTEND)
13561337
IsRelationExtensionLockHeld=acquired;
1357-
elseif (LOCALLOCK_LOCKTAG(*locallock)==LOCKTAG_PAGE)
1358-
IsPageLockHeld=acquired;
1359-
13601338
#endif
13611339
}
13621340

@@ -1482,11 +1460,9 @@ LockCheckConflicts(LockMethod lockMethodTable,
14821460
}
14831461

14841462
/*
1485-
* The relation extension or page lock conflict even between the group
1486-
* members.
1463+
* The relation extension lock conflict even between the group members.
14871464
*/
1488-
if (LOCK_LOCKTAG(*lock)==LOCKTAG_RELATION_EXTEND||
1489-
(LOCK_LOCKTAG(*lock)==LOCKTAG_PAGE))
1465+
if (LOCK_LOCKTAG(*lock)==LOCKTAG_RELATION_EXTEND)
14901466
{
14911467
PROCLOCK_PRINT("LockCheckConflicts: conflicting (group)",
14921468
proclock);

‎src/backend/storage/lmgr/proc.c

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1080,12 +1080,12 @@ ProcSleep(LOCALLOCK *locallock, LockMethod lockMethodTable)
10801080
/*
10811081
* If group locking is in use, locks held by members of my locking group
10821082
* need to be included in myHeldLocks. This is not required for relation
1083-
* extensionor page lockswhich conflict among group members. However,
1084-
*includingthem in myHeldLocks will give group members the priority to
1085-
*get thoselocks as compared to other backends which are also trying to
1086-
*acquirethose locks. OTOH, we can avoid giving priority to group
1087-
*members forthat kind of locks, but there doesn't appear to be a clear
1088-
*advantage ofthe same.
1083+
* extensionlockwhich conflict among group members. However, including
1084+
* them in myHeldLocks will give group members the priority to get those
1085+
* locks as compared to other backends which are also trying to acquire
1086+
* those locks. OTOH, we can avoid giving priority to group members for
1087+
* that kind of locks, but there doesn't appear to be a clear advantage of
1088+
* the same.
10891089
*/
10901090
if (leader!=NULL)
10911091
{

0 commit comments

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