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
Appearance settings

Commit 2346df6

Browse filesBrowse files
robertmhaasrobertmhaas
committed
Allow a no-wait lock acquisition to succeed in more cases.
We don't determine the position at which a process waiting for a lock should insert itself into the wait queue until we reach ProcSleep(), and we may at that point discover that we must insert ourselves ahead of everyone who wants a conflicting lock, in which case we obtain the lock immediately. Up until now, a no-wait lock acquisition would fail in such cases, erroneously claiming that the lock couldn't be obtained immediately. Fix that by trying ProcSleep even in the no-wait case. No back-patch for now, because I'm treating this as an improvement to the existing no-wait feature. It could instead be argued that it's a bug fix, on the theory that there should never be any case whatsoever where no-wait fails to obtain a lock that would have been obtained immediately without no-wait, but I'm reluctant to interpret the semantics of no-wait that strictly. Robert Haas and Jingxian Li Discussion: http://postgr.es/m/CA+TgmobCH-kMXGVpb0BB-iNMdtcNkTvcZ4JBxDJows3kYM+GDg@mail.gmail.com
1 parent c20d90a commit 2346df6
Copy full SHA for 2346df6

File tree

Expand file treeCollapse file tree

6 files changed

+128
-53
lines changed
Filter options
Expand file treeCollapse file tree

6 files changed

+128
-53
lines changed

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

Copy file name to clipboardExpand all lines: src/backend/storage/lmgr/lock.c
+69-50Lines changed: 69 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -360,7 +360,8 @@ static PROCLOCK *SetupLockInTable(LockMethod lockMethodTable, PGPROC *proc,
360360
static void GrantLockLocal(LOCALLOCK *locallock, ResourceOwner owner);
361361
static void BeginStrongLockAcquire(LOCALLOCK *locallock, uint32 fasthashcode);
362362
static void FinishStrongLockAcquire(void);
363-
static void WaitOnLock(LOCALLOCK *locallock, ResourceOwner owner);
363+
static void WaitOnLock(LOCALLOCK *locallock, ResourceOwner owner,
364+
bool dontWait);
364365
static void ReleaseLockIfHeld(LOCALLOCK *locallock, bool sessionLock);
365366
static void LockReassignOwner(LOCALLOCK *locallock, ResourceOwner parent);
366367
static bool UnGrantLock(LOCK *lock, LOCKMODE lockmode,
@@ -1024,50 +1025,15 @@ LockAcquireExtended(const LOCKTAG *locktag,
10241025
}
10251026
else
10261027
{
1027-
/*
1028-
* We can't acquire the lock immediately. If caller specified no
1029-
* blocking, remove useless table entries and return
1030-
* LOCKACQUIRE_NOT_AVAIL without waiting.
1031-
*/
1032-
if (dontWait)
1033-
{
1034-
AbortStrongLockAcquire();
1035-
if (proclock->holdMask == 0)
1036-
{
1037-
uint32 proclock_hashcode;
1038-
1039-
proclock_hashcode = ProcLockHashCode(&proclock->tag, hashcode);
1040-
dlist_delete(&proclock->lockLink);
1041-
dlist_delete(&proclock->procLink);
1042-
if (!hash_search_with_hash_value(LockMethodProcLockHash,
1043-
&(proclock->tag),
1044-
proclock_hashcode,
1045-
HASH_REMOVE,
1046-
NULL))
1047-
elog(PANIC, "proclock table corrupted");
1048-
}
1049-
else
1050-
PROCLOCK_PRINT("LockAcquire: NOWAIT", proclock);
1051-
lock->nRequested--;
1052-
lock->requested[lockmode]--;
1053-
LOCK_PRINT("LockAcquire: conditional lock failed", lock, lockmode);
1054-
Assert((lock->nRequested > 0) && (lock->requested[lockmode] >= 0));
1055-
Assert(lock->nGranted <= lock->nRequested);
1056-
LWLockRelease(partitionLock);
1057-
if (locallock->nLocks == 0)
1058-
RemoveLocalLock(locallock);
1059-
if (locallockp)
1060-
*locallockp = NULL;
1061-
return LOCKACQUIRE_NOT_AVAIL;
1062-
}
1063-
10641028
/*
10651029
* Set bitmask of locks this process already holds on this object.
10661030
*/
10671031
MyProc->heldLocks = proclock->holdMask;
10681032

10691033
/*
1070-
* Sleep till someone wakes me up.
1034+
* Sleep till someone wakes me up. We do this even in the dontWait
1035+
* case, beause while trying to go to sleep, we may discover that we
1036+
* can acquire the lock immediately after all.
10711037
*/
10721038

10731039
TRACE_POSTGRESQL_LOCK_WAIT_START(locktag->locktag_field1,
@@ -1077,7 +1043,7 @@ LockAcquireExtended(const LOCKTAG *locktag,
10771043
locktag->locktag_type,
10781044
lockmode);
10791045

1080-
WaitOnLock(locallock, owner);
1046+
WaitOnLock(locallock, owner, dontWait);
10811047

10821048
TRACE_POSTGRESQL_LOCK_WAIT_DONE(locktag->locktag_field1,
10831049
locktag->locktag_field2,
@@ -1093,17 +1059,63 @@ LockAcquireExtended(const LOCKTAG *locktag,
10931059
*/
10941060

10951061
/*
1096-
* Check the proclock entry status, in case something in the ipc
1097-
* communication doesn't work correctly.
1062+
* Check the proclock entry status. If dontWait = true, this is an
1063+
* expected case; otherwise, it will open happen if something in the
1064+
* ipc communication doesn't work correctly.
10981065
*/
10991066
if (!(proclock->holdMask & LOCKBIT_ON(lockmode)))
11001067
{
11011068
AbortStrongLockAcquire();
1102-
PROCLOCK_PRINT("LockAcquire: INCONSISTENT", proclock);
1103-
LOCK_PRINT("LockAcquire: INCONSISTENT", lock, lockmode);
1104-
/* Should we retry ? */
1105-
LWLockRelease(partitionLock);
1106-
elog(ERROR, "LockAcquire failed");
1069+
1070+
if (dontWait)
1071+
{
1072+
/*
1073+
* We can't acquire the lock immediately. If caller specified
1074+
* no blocking, remove useless table entries and return
1075+
* LOCKACQUIRE_NOT_AVAIL without waiting.
1076+
*/
1077+
if (proclock->holdMask == 0)
1078+
{
1079+
uint32 proclock_hashcode;
1080+
1081+
proclock_hashcode = ProcLockHashCode(&proclock->tag,
1082+
hashcode);
1083+
dlist_delete(&proclock->lockLink);
1084+
dlist_delete(&proclock->procLink);
1085+
if (!hash_search_with_hash_value(LockMethodProcLockHash,
1086+
&(proclock->tag),
1087+
proclock_hashcode,
1088+
HASH_REMOVE,
1089+
NULL))
1090+
elog(PANIC, "proclock table corrupted");
1091+
}
1092+
else
1093+
PROCLOCK_PRINT("LockAcquire: NOWAIT", proclock);
1094+
lock->nRequested--;
1095+
lock->requested[lockmode]--;
1096+
LOCK_PRINT("LockAcquire: conditional lock failed",
1097+
lock, lockmode);
1098+
Assert((lock->nRequested > 0) &&
1099+
(lock->requested[lockmode] >= 0));
1100+
Assert(lock->nGranted <= lock->nRequested);
1101+
LWLockRelease(partitionLock);
1102+
if (locallock->nLocks == 0)
1103+
RemoveLocalLock(locallock);
1104+
if (locallockp)
1105+
*locallockp = NULL;
1106+
return LOCKACQUIRE_NOT_AVAIL;
1107+
}
1108+
else
1109+
{
1110+
/*
1111+
* We should have gotten the lock, but somehow that didn't
1112+
* happen. If we get here, it's a bug.
1113+
*/
1114+
PROCLOCK_PRINT("LockAcquire: INCONSISTENT", proclock);
1115+
LOCK_PRINT("LockAcquire: INCONSISTENT", lock, lockmode);
1116+
LWLockRelease(partitionLock);
1117+
elog(ERROR, "LockAcquire failed");
1118+
}
11071119
}
11081120
PROCLOCK_PRINT("LockAcquire: granted", proclock);
11091121
LOCK_PRINT("LockAcquire: granted", lock, lockmode);
@@ -1777,10 +1789,11 @@ MarkLockClear(LOCALLOCK *locallock)
17771789
* Caller must have set MyProc->heldLocks to reflect locks already held
17781790
* on the lockable object by this process.
17791791
*
1780-
* The appropriate partition lock must be held at entry.
1792+
* The appropriate partition lock must be held at entry, and will still be
1793+
* held at exit.
17811794
*/
17821795
static void
1783-
WaitOnLock(LOCALLOCK *locallock, ResourceOwner owner)
1796+
WaitOnLock(LOCALLOCK *locallock, ResourceOwner owner, bool dontWait)
17841797
{
17851798
LOCKMETHODID lockmethodid = LOCALLOCK_LOCKMETHOD(*locallock);
17861799
LockMethod lockMethodTable = LockMethods[lockmethodid];
@@ -1813,8 +1826,14 @@ WaitOnLock(LOCALLOCK *locallock, ResourceOwner owner)
18131826
*/
18141827
PG_TRY();
18151828
{
1816-
if (ProcSleep(locallock, lockMethodTable) != PROC_WAIT_STATUS_OK)
1829+
/*
1830+
* If dontWait = true, we handle success and failure in the same way
1831+
* here. The caller will be able to sort out what has happened.
1832+
*/
1833+
if (ProcSleep(locallock, lockMethodTable, dontWait) != PROC_WAIT_STATUS_OK
1834+
&& !dontWait)
18171835
{
1836+
18181837
/*
18191838
* We failed as a result of a deadlock, see CheckDeadLock(). Quit
18201839
* now.

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

Copy file name to clipboardExpand all lines: src/backend/storage/lmgr/proc.c
+18-2Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1043,18 +1043,27 @@ AuxiliaryPidGetProc(int pid)
10431043
* Caller must have set MyProc->heldLocks to reflect locks already held
10441044
* on the lockable object by this process (under all XIDs).
10451045
*
1046+
* It's not actually guaranteed that we need to wait when this function is
1047+
* called, because it could be that when we try to find a position at which
1048+
* to insert ourself into the wait queue, we discover that we must be inserted
1049+
* ahead of everyone who wants a lock that conflict with ours. In that case,
1050+
* we get the lock immediately. Beause of this, it's sensible for this function
1051+
* to have a dontWait argument, despite the name.
1052+
*
10461053
* The lock table's partition lock must be held at entry, and will be held
10471054
* at exit.
10481055
*
1049-
* Result: PROC_WAIT_STATUS_OK if we acquired the lock, PROC_WAIT_STATUS_ERROR if not (deadlock).
1056+
* Result: PROC_WAIT_STATUS_OK if we acquired the lock, PROC_WAIT_STATUS_ERROR
1057+
* if not (if dontWait = true, this is a deadlock; if dontWait = false, we
1058+
* would have had to wait).
10501059
*
10511060
* ASSUME: that no one will fiddle with the queue until after
10521061
* we release the partition lock.
10531062
*
10541063
* NOTES: The process queue is now a priority queue for locking.
10551064
*/
10561065
ProcWaitStatus
1057-
ProcSleep(LOCALLOCK *locallock, LockMethod lockMethodTable)
1066+
ProcSleep(LOCALLOCK *locallock, LockMethod lockMethodTable, bool dontWait)
10581067
{
10591068
LOCKMODE lockmode = locallock->tag.mode;
10601069
LOCK *lock = locallock->lock;
@@ -1167,6 +1176,13 @@ ProcSleep(LOCALLOCK *locallock, LockMethod lockMethodTable)
11671176
}
11681177
}
11691178

1179+
/*
1180+
* At this point we know that we'd really need to sleep. If we've been
1181+
* commanded not to do that, bail out.
1182+
*/
1183+
if (dontWait)
1184+
return PROC_WAIT_STATUS_ERROR;
1185+
11701186
/*
11711187
* Insert self into queue, at the position determined above.
11721188
*/

‎src/include/storage/proc.h

Copy file name to clipboardExpand all lines: src/include/storage/proc.h
+3-1Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -471,7 +471,9 @@ extern int GetStartupBufferPinWaitBufId(void);
471471
extern bool HaveNFreeProcs(int n, int *nfree);
472472
extern void ProcReleaseLocks(bool isCommit);
473473

474-
extern ProcWaitStatus ProcSleep(LOCALLOCK *locallock, LockMethod lockMethodTable);
474+
extern ProcWaitStatus ProcSleep(LOCALLOCK *locallock,
475+
LockMethod lockMethodTable,
476+
bool dontWait);
475477
extern void ProcWakeup(PGPROC *proc, ProcWaitStatus waitStatus);
476478
extern void ProcLockWakeup(LockMethod lockMethodTable, LOCK *lock);
477479
extern void CheckDeadLockAlert(void);

‎src/test/isolation/expected/lock-nowait.out

Copy file name to clipboard
+9Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
Parsed test spec with 2 sessions
2+
3+
starting permutation: s1a s2a s1b s1c s2c
4+
step s1a: LOCK TABLE a1 IN ACCESS EXCLUSIVE MODE;
5+
step s2a: LOCK TABLE a1 IN EXCLUSIVE MODE; <waiting ...>
6+
step s1b: LOCK TABLE a1 IN SHARE ROW EXCLUSIVE MODE NOWAIT;
7+
step s1c: COMMIT;
8+
step s2a: <... completed>
9+
step s2c: COMMIT;

‎src/test/isolation/isolation_schedule

Copy file name to clipboardExpand all lines: src/test/isolation/isolation_schedule
+1Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,3 +111,4 @@ test: serializable-parallel
111111
test: serializable-parallel-2
112112
test: serializable-parallel-3
113113
test: matview-write-skew
114+
test: lock-nowait

‎src/test/isolation/specs/lock-nowait.spec

Copy file name to clipboard
+28Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
# While requesting nowait lock, if the lock requested should
2+
# be inserted in front of some waiter, check to see if the lock
3+
# conflicts with already-held locks or the requests before
4+
# the waiter. If not, then just grant myself the requested
5+
# lock immediately. Test this scenario.
6+
7+
setup
8+
{
9+
CREATE TABLE a1 ();
10+
}
11+
12+
teardown
13+
{
14+
DROP TABLE a1;
15+
}
16+
17+
session s1
18+
setup { BEGIN; }
19+
step s1a { LOCK TABLE a1 IN ACCESS EXCLUSIVE MODE; }
20+
step s1b { LOCK TABLE a1 IN SHARE ROW EXCLUSIVE MODE NOWAIT; }
21+
step s1c { COMMIT; }
22+
23+
session s2
24+
setup { BEGIN; }
25+
step s2a { LOCK TABLE a1 IN EXCLUSIVE MODE; }
26+
step s2c { COMMIT; }
27+
28+
permutation s1a s2a s1b s1c s2c

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.