Skip to content

Commit 3c0fd64

Browse files
hlinnakahlinnaka
committed
Split ProcSleep function into JoinWaitQueue and ProcSleep
Split ProcSleep into two functions: JoinWaitQueue and ProcSleep. JoinWaitQueue is called while holding the partition lock, and inserts the current process to the wait queue, while ProcSleep() does the actual sleeping. ProcSleep() is now called without holding the partition lock, and it no longer re-acquires the partition lock before returning. That makes the wakeup a little cheaper. Once upon a time, re-acquiring the partition lock was needed to prevent a signal handler from longjmping out at a bad time, but these days our signal handlers just set flags, and longjmping can only happen at points where we explicitly run CHECK_FOR_INTERRUPTS(). If JoinWaitQueue detects an "early deadlock" before even joining the wait queue, it returns without changing the shared lock entry, leaving the cleanup of the shared lock entry to the caller. This makes the handling of an early deadlock the same as the dontWait=true case. One small user-visible side-effect of this refactoring is that we now only set the 'ps' title to say "waiting" when we actually enter the sleep, not when the lock is skipped because dontWait=true, or when a deadlock is detected early before entering the sleep. This eliminates the 'lockAwaited' global variable in proc.c, which was largely redundant with 'awaitedLock' in lock.c Note: Updating the local lock table is now the caller's responsibility. JoinWaitQueue and ProcSleep are now only responsible for modifying the shared state. Seems a little nicer that way. Based on Thomas Munro's earlier patch and observation that ProcSleep doesn't really need to re-acquire the partition lock. Reviewed-by: Maxim Orlov Discussion: https://www.postgresql.org/message-id/7c2090cd-a72a-4e34-afaa-6dd2ef31440e@iki.fi
1 parent 0b1765d commit 3c0fd64

File tree

5 files changed

+185
-186
lines changed

5 files changed

+185
-186
lines changed

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

Lines changed: 112 additions & 104 deletions
Original file line numberDiff line numberDiff line change
@@ -409,8 +409,7 @@ static PROCLOCK *SetupLockInTable(LockMethod lockMethodTable, PGPROC *proc,
409409
static void GrantLockLocal(LOCALLOCK *locallock, ResourceOwner owner);
410410
static void BeginStrongLockAcquire(LOCALLOCK *locallock, uint32 fasthashcode);
411411
static void FinishStrongLockAcquire(void);
412-
static void WaitOnLock(LOCALLOCK *locallock, ResourceOwner owner,
413-
bool dontWait);
412+
static ProcWaitStatus WaitOnLock(LOCALLOCK *locallock, ResourceOwner owner);
414413
static void ReleaseLockIfHeld(LOCALLOCK *locallock, bool sessionLock);
415414
static void LockReassignOwner(LOCALLOCK *locallock, ResourceOwner parent);
416415
static bool UnGrantLock(LOCK *lock, LOCKMODE lockmode,
@@ -843,6 +842,7 @@ LockAcquireExtended(const LOCKTAG *locktag,
843842
uint32 hashcode;
844843
LWLock *partitionLock;
845844
bool found_conflict;
845+
ProcWaitStatus waitResult;
846846
bool log_lock = false;
847847

848848
if (lockmethodid <= 0 || lockmethodid >= lengthof(LockMethods))
@@ -1091,94 +1091,115 @@ LockAcquireExtended(const LOCKTAG *locktag,
10911091
{
10921092
/* No conflict with held or previously requested locks */
10931093
GrantLock(lock, proclock, lockmode);
1094-
GrantLockLocal(locallock, owner);
1094+
waitResult = PROC_WAIT_STATUS_OK;
10951095
}
10961096
else
10971097
{
10981098
/*
1099-
* Sleep till someone wakes me up. We do this even in the dontWait
1100-
* case, because while trying to go to sleep, we may discover that we
1101-
* can acquire the lock immediately after all.
1099+
* Join the lock's wait queue. We call this even in the dontWait
1100+
* case, because JoinWaitQueue() may discover that we can acquire the
1101+
* lock immediately after all.
1102+
*/
1103+
waitResult = JoinWaitQueue(locallock, lockMethodTable, dontWait);
1104+
}
1105+
1106+
if (waitResult == PROC_WAIT_STATUS_ERROR)
1107+
{
1108+
/*
1109+
* We're not getting the lock because a deadlock was detected already
1110+
* while trying to join the wait queue, or because we would have to
1111+
* wait but the caller requested no blocking.
1112+
*
1113+
* Undo the changes to shared entries before releasing the partition
1114+
* lock.
11021115
*/
1103-
WaitOnLock(locallock, owner, dontWait);
1116+
AbortStrongLockAcquire();
1117+
1118+
if (proclock->holdMask == 0)
1119+
{
1120+
uint32 proclock_hashcode;
1121+
1122+
proclock_hashcode = ProcLockHashCode(&proclock->tag,
1123+
hashcode);
1124+
dlist_delete(&proclock->lockLink);
1125+
dlist_delete(&proclock->procLink);
1126+
if (!hash_search_with_hash_value(LockMethodProcLockHash,
1127+
&(proclock->tag),
1128+
proclock_hashcode,
1129+
HASH_REMOVE,
1130+
NULL))
1131+
elog(PANIC, "proclock table corrupted");
1132+
}
1133+
else
1134+
PROCLOCK_PRINT("LockAcquire: did not join wait queue", proclock);
1135+
lock->nRequested--;
1136+
lock->requested[lockmode]--;
1137+
LOCK_PRINT("LockAcquire: did not join wait queue",
1138+
lock, lockmode);
1139+
Assert((lock->nRequested > 0) &&
1140+
(lock->requested[lockmode] >= 0));
1141+
Assert(lock->nGranted <= lock->nRequested);
1142+
LWLockRelease(partitionLock);
1143+
if (locallock->nLocks == 0)
1144+
RemoveLocalLock(locallock);
1145+
1146+
if (dontWait)
1147+
{
1148+
if (locallockp)
1149+
*locallockp = NULL;
1150+
return LOCKACQUIRE_NOT_AVAIL;
1151+
}
1152+
else
1153+
{
1154+
DeadLockReport();
1155+
/* DeadLockReport() will not return */
1156+
}
1157+
}
1158+
1159+
/*
1160+
* We are now in the lock queue, or the lock was already granted. If
1161+
* queued, go to sleep.
1162+
*/
1163+
if (waitResult == PROC_WAIT_STATUS_WAITING)
1164+
{
1165+
Assert(!dontWait);
1166+
PROCLOCK_PRINT("LockAcquire: sleeping on lock", proclock);
1167+
LOCK_PRINT("LockAcquire: sleeping on lock", lock, lockmode);
1168+
LWLockRelease(partitionLock);
1169+
1170+
waitResult = WaitOnLock(locallock, owner);
11041171

11051172
/*
11061173
* NOTE: do not do any material change of state between here and
11071174
* return. All required changes in locktable state must have been
11081175
* done when the lock was granted to us --- see notes in WaitOnLock.
11091176
*/
11101177

1111-
/*
1112-
* Check the proclock entry status. If dontWait = true, this is an
1113-
* expected case; otherwise, it will only happen if something in the
1114-
* ipc communication doesn't work correctly.
1115-
*/
1116-
if (!(proclock->holdMask & LOCKBIT_ON(lockmode)))
1178+
if (waitResult == PROC_WAIT_STATUS_ERROR)
11171179
{
1118-
AbortStrongLockAcquire();
1119-
1120-
if (dontWait)
1121-
{
1122-
/*
1123-
* We can't acquire the lock immediately. If caller specified
1124-
* no blocking, remove useless table entries and return
1125-
* LOCKACQUIRE_NOT_AVAIL without waiting.
1126-
*/
1127-
if (proclock->holdMask == 0)
1128-
{
1129-
uint32 proclock_hashcode;
1130-
1131-
proclock_hashcode = ProcLockHashCode(&proclock->tag,
1132-
hashcode);
1133-
dlist_delete(&proclock->lockLink);
1134-
dlist_delete(&proclock->procLink);
1135-
if (!hash_search_with_hash_value(LockMethodProcLockHash,
1136-
&(proclock->tag),
1137-
proclock_hashcode,
1138-
HASH_REMOVE,
1139-
NULL))
1140-
elog(PANIC, "proclock table corrupted");
1141-
}
1142-
else
1143-
PROCLOCK_PRINT("LockAcquire: NOWAIT", proclock);
1144-
lock->nRequested--;
1145-
lock->requested[lockmode]--;
1146-
LOCK_PRINT("LockAcquire: conditional lock failed",
1147-
lock, lockmode);
1148-
Assert((lock->nRequested > 0) &&
1149-
(lock->requested[lockmode] >= 0));
1150-
Assert(lock->nGranted <= lock->nRequested);
1151-
LWLockRelease(partitionLock);
1152-
if (locallock->nLocks == 0)
1153-
RemoveLocalLock(locallock);
1154-
if (locallockp)
1155-
*locallockp = NULL;
1156-
return LOCKACQUIRE_NOT_AVAIL;
1157-
}
1158-
else
1159-
{
1160-
/*
1161-
* We should have gotten the lock, but somehow that didn't
1162-
* happen. If we get here, it's a bug.
1163-
*/
1164-
PROCLOCK_PRINT("LockAcquire: INCONSISTENT", proclock);
1165-
LOCK_PRINT("LockAcquire: INCONSISTENT", lock, lockmode);
1166-
LWLockRelease(partitionLock);
1167-
elog(ERROR, "LockAcquire failed");
1168-
}
1180+
/*
1181+
* We failed as a result of a deadlock, see CheckDeadLock(). Quit
1182+
* now.
1183+
*/
1184+
Assert(!dontWait);
1185+
DeadLockReport();
1186+
/* DeadLockReport() will not return */
11691187
}
1170-
PROCLOCK_PRINT("LockAcquire: granted", proclock);
1171-
LOCK_PRINT("LockAcquire: granted", lock, lockmode);
11721188
}
1189+
else
1190+
LWLockRelease(partitionLock);
1191+
Assert(waitResult == PROC_WAIT_STATUS_OK);
1192+
1193+
/* The lock was granted to us. Update the local lock entry accordingly */
1194+
Assert((proclock->holdMask & LOCKBIT_ON(lockmode)) != 0);
1195+
GrantLockLocal(locallock, owner);
11731196

11741197
/*
11751198
* Lock state is fully up-to-date now; if we error out after this, no
11761199
* special error cleanup is required.
11771200
*/
11781201
FinishStrongLockAcquire();
11791202

1180-
LWLockRelease(partitionLock);
1181-
11821203
/*
11831204
* Emit a WAL record if acquisition of this lock needs to be replayed in a
11841205
* standby server.
@@ -1819,6 +1840,15 @@ GrantAwaitedLock(void)
18191840
GrantLockLocal(awaitedLock, awaitedOwner);
18201841
}
18211842

1843+
/*
1844+
* GetAwaitedLock -- Return the lock we're currently doing WaitOnLock on.
1845+
*/
1846+
LOCALLOCK *
1847+
GetAwaitedLock(void)
1848+
{
1849+
return awaitedLock;
1850+
}
1851+
18221852
/*
18231853
* MarkLockClear -- mark an acquired lock as "clear"
18241854
*
@@ -1836,14 +1866,12 @@ MarkLockClear(LOCALLOCK *locallock)
18361866
/*
18371867
* WaitOnLock -- wait to acquire a lock
18381868
*
1839-
* The appropriate partition lock must be held at entry, and will still be
1840-
* held at exit.
1869+
* This is a wrapper around ProcSleep, with extra tracing and bookkeeping.
18411870
*/
1842-
static void
1843-
WaitOnLock(LOCALLOCK *locallock, ResourceOwner owner, bool dontWait)
1871+
static ProcWaitStatus
1872+
WaitOnLock(LOCALLOCK *locallock, ResourceOwner owner)
18441873
{
1845-
LOCKMETHODID lockmethodid = LOCALLOCK_LOCKMETHOD(*locallock);
1846-
LockMethod lockMethodTable = LockMethods[lockmethodid];
1874+
ProcWaitStatus result;
18471875

18481876
TRACE_POSTGRESQL_LOCK_WAIT_START(locallock->tag.lock.locktag_field1,
18491877
locallock->tag.lock.locktag_field2,
@@ -1852,12 +1880,13 @@ WaitOnLock(LOCALLOCK *locallock, ResourceOwner owner, bool dontWait)
18521880
locallock->tag.lock.locktag_type,
18531881
locallock->tag.mode);
18541882

1855-
LOCK_PRINT("WaitOnLock: sleeping on lock",
1856-
locallock->lock, locallock->tag.mode);
1857-
18581883
/* adjust the process title to indicate that it's waiting */
18591884
set_ps_display_suffix("waiting");
18601885

1886+
/*
1887+
* Record the fact that we are waiting for a lock, so that
1888+
* LockErrorCleanup will clean up if cancel/die happens.
1889+
*/
18611890
awaitedLock = locallock;
18621891
awaitedOwner = owner;
18631892

@@ -1880,30 +1909,7 @@ WaitOnLock(LOCALLOCK *locallock, ResourceOwner owner, bool dontWait)
18801909
*/
18811910
PG_TRY();
18821911
{
1883-
/*
1884-
* If dontWait = true, we handle success and failure in the same way
1885-
* here. The caller will be able to sort out what has happened.
1886-
*/
1887-
if (ProcSleep(locallock, lockMethodTable, dontWait) != PROC_WAIT_STATUS_OK
1888-
&& !dontWait)
1889-
{
1890-
1891-
/*
1892-
* We failed as a result of a deadlock, see CheckDeadLock(). Quit
1893-
* now.
1894-
*/
1895-
awaitedLock = NULL;
1896-
LOCK_PRINT("WaitOnLock: aborting on lock",
1897-
locallock->lock, locallock->tag.mode);
1898-
LWLockRelease(LockHashPartitionLock(locallock->hashcode));
1899-
1900-
/*
1901-
* Now that we aren't holding the partition lock, we can give an
1902-
* error report including details about the detected deadlock.
1903-
*/
1904-
DeadLockReport();
1905-
/* not reached */
1906-
}
1912+
result = ProcSleep(locallock);
19071913
}
19081914
PG_CATCH();
19091915
{
@@ -1917,20 +1923,22 @@ WaitOnLock(LOCALLOCK *locallock, ResourceOwner owner, bool dontWait)
19171923
}
19181924
PG_END_TRY();
19191925

1926+
/*
1927+
* We no longer want LockErrorCleanup to do anything.
1928+
*/
19201929
awaitedLock = NULL;
19211930

19221931
/* reset ps display to remove the suffix */
19231932
set_ps_display_remove_suffix();
19241933

1925-
LOCK_PRINT("WaitOnLock: wakeup on lock",
1926-
locallock->lock, locallock->tag.mode);
1927-
19281934
TRACE_POSTGRESQL_LOCK_WAIT_DONE(locallock->tag.lock.locktag_field1,
19291935
locallock->tag.lock.locktag_field2,
19301936
locallock->tag.lock.locktag_field3,
19311937
locallock->tag.lock.locktag_field4,
19321938
locallock->tag.lock.locktag_type,
19331939
locallock->tag.mode);
1940+
1941+
return result;
19341942
}
19351943

19361944
/*

0 commit comments

Comments
 (0)