From 049fa9519d14b3b01bffbd219ebbc8340987f90e Mon Sep 17 00:00:00 2001 From: Peter Eisentraut Date: Sun, 29 Dec 2019 09:09:20 +0100 Subject: [PATCH 2/4] Remove STATUS_WAITING Add a separate enum for use in the locking APIs, which were the only user. --- src/backend/access/transam/twophase.c | 2 +- src/backend/storage/lmgr/lock.c | 8 ++--- src/backend/storage/lmgr/proc.c | 46 +++++++++++++-------------- src/include/c.h | 1 - src/include/storage/proc.h | 13 ++++++-- 5 files changed, 38 insertions(+), 32 deletions(-) diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c index 529976885f..f8582d2cbb 100644 --- a/src/backend/access/transam/twophase.c +++ b/src/backend/access/transam/twophase.c @@ -460,7 +460,7 @@ MarkAsPreparingGuts(GlobalTransaction gxact, TransactionId xid, const char *gid, MemSet(proc, 0, sizeof(PGPROC)); proc->pgprocno = gxact->pgprocno; SHMQueueElemInit(&(proc->links)); - proc->waitStatus = STATUS_OK; + proc->waitStatus = PROC_WAIT_STATUS_OK; /* We set up the gxact's VXID as InvalidBackendId/XID */ proc->lxid = (LocalTransactionId) xid; pgxact->xid = xid; diff --git a/src/backend/storage/lmgr/lock.c b/src/backend/storage/lmgr/lock.c index e2c728a97d..33c9f471aa 100644 --- a/src/backend/storage/lmgr/lock.c +++ b/src/backend/storage/lmgr/lock.c @@ -1763,7 +1763,7 @@ WaitOnLock(LOCALLOCK *locallock, ResourceOwner owner) */ PG_TRY(); { - if (ProcSleep(locallock, lockMethodTable) != STATUS_OK) + if (ProcSleep(locallock, lockMethodTable) != PROC_WAIT_STATUS_OK) { /* * We failed as a result of a deadlock, see CheckDeadLock(). Quit @@ -1814,7 +1814,7 @@ WaitOnLock(LOCALLOCK *locallock, ResourceOwner owner) /* * Remove a proc from the wait-queue it is on (caller must know it is on one). * This is only used when the proc has failed to get the lock, so we set its - * waitStatus to STATUS_ERROR. + * waitStatus to PROC_WAIT_STATUS_ERROR. * * Appropriate partition lock must be held by caller. Also, caller is * responsible for signaling the proc if needed. @@ -1830,7 +1830,7 @@ RemoveFromWaitQueue(PGPROC *proc, uint32 hashcode) LOCKMETHODID lockmethodid = LOCK_LOCKMETHOD(*waitLock); /* Make sure proc is waiting */ - Assert(proc->waitStatus == STATUS_WAITING); + Assert(proc->waitStatus == PROC_WAIT_STATUS_WAITING); Assert(proc->links.next != NULL); Assert(waitLock); Assert(waitLock->waitProcs.size > 0); @@ -1853,7 +1853,7 @@ RemoveFromWaitQueue(PGPROC *proc, uint32 hashcode) /* Clean up the proc's own state, and pass it the ok/fail signal */ proc->waitLock = NULL; proc->waitProcLock = NULL; - proc->waitStatus = STATUS_ERROR; + proc->waitStatus = PROC_WAIT_STATUS_ERROR; /* * Delete the proclock immediately if it represents no already-held locks. diff --git a/src/backend/storage/lmgr/proc.c b/src/backend/storage/lmgr/proc.c index dbb541757f..cdd6c8893a 100644 --- a/src/backend/storage/lmgr/proc.c +++ b/src/backend/storage/lmgr/proc.c @@ -383,7 +383,7 @@ InitProcess(void) * initialized by InitProcGlobal. */ SHMQueueElemInit(&(MyProc->links)); - MyProc->waitStatus = STATUS_OK; + MyProc->waitStatus = PROC_WAIT_STATUS_OK; MyProc->lxid = InvalidLocalTransactionId; MyProc->fpVXIDLock = false; MyProc->fpLocalTransactionId = InvalidLocalTransactionId; @@ -567,7 +567,7 @@ InitAuxiliaryProcess(void) * initialized by InitProcGlobal. */ SHMQueueElemInit(&(MyProc->links)); - MyProc->waitStatus = STATUS_OK; + MyProc->waitStatus = PROC_WAIT_STATUS_OK; MyProc->lxid = InvalidLocalTransactionId; MyProc->fpVXIDLock = false; MyProc->fpLocalTransactionId = InvalidLocalTransactionId; @@ -755,7 +755,7 @@ LockErrorCleanup(void) * did grant us the lock, we'd better remember it in our local lock * table. */ - if (MyProc->waitStatus == STATUS_OK) + if (MyProc->waitStatus == PROC_WAIT_STATUS_OK) GrantAwaitedLock(); } @@ -1051,14 +1051,14 @@ ProcQueueInit(PROC_QUEUE *queue) * The lock table's partition lock must be held at entry, and will be held * at exit. * - * Result: STATUS_OK if we acquired the lock, STATUS_ERROR if not (deadlock). + * Result: PROC_WAIT_STATUS_OK if we acquired the lock, PROC_WAIT_STATUS_ERROR if not (deadlock). * * ASSUME: that no one will fiddle with the queue until after * we release the partition lock. * * NOTES: The process queue is now a priority queue for locking. */ -int +ProcWaitStatus ProcSleep(LOCALLOCK *locallock, LockMethod lockMethodTable) { LOCKMODE lockmode = locallock->tag.mode; @@ -1070,7 +1070,7 @@ ProcSleep(LOCALLOCK *locallock, LockMethod lockMethodTable) LOCKMASK myHeldLocks = MyProc->heldLocks; bool early_deadlock = false; bool allow_autovacuum_cancel = true; - int myWaitStatus; + ProcWaitStatus myWaitStatus; PGPROC *proc; PGPROC *leader = MyProc->lockGroupLeader; int i; @@ -1155,7 +1155,7 @@ ProcSleep(LOCALLOCK *locallock, LockMethod lockMethodTable) /* Skip the wait and just grant myself the lock. */ GrantLock(lock, proclock, lockmode); GrantAwaitedLock(); - return STATUS_OK; + return PROC_WAIT_STATUS_OK; } /* Break out of loop to put myself before him */ break; @@ -1189,7 +1189,7 @@ ProcSleep(LOCALLOCK *locallock, LockMethod lockMethodTable) MyProc->waitProcLock = proclock; MyProc->waitLockMode = lockmode; - MyProc->waitStatus = STATUS_WAITING; + MyProc->waitStatus = PROC_WAIT_STATUS_WAITING; /* * If we detected deadlock, give up without waiting. This must agree with @@ -1198,7 +1198,7 @@ ProcSleep(LOCALLOCK *locallock, LockMethod lockMethodTable) if (early_deadlock) { RemoveFromWaitQueue(MyProc, hashcode); - return STATUS_ERROR; + return PROC_WAIT_STATUS_ERROR; } /* mark that we are waiting for a lock */ @@ -1230,7 +1230,7 @@ ProcSleep(LOCALLOCK *locallock, LockMethod lockMethodTable) /* * Set timer so we can wake up after awhile and check for a deadlock. If a * deadlock is detected, the handler sets MyProc->waitStatus = - * STATUS_ERROR, allowing us to know that we must report failure rather + * PROC_WAIT_STATUS_ERROR, allowing us to know that we must report failure rather * than success. * * By delaying the check until we've waited for a bit, we can avoid @@ -1296,11 +1296,11 @@ ProcSleep(LOCALLOCK *locallock, LockMethod lockMethodTable) } /* - * waitStatus could change from STATUS_WAITING to something else + * waitStatus could change from PROC_WAIT_STATUS_WAITING to something else * asynchronously. Read it just once per loop to prevent surprising * behavior (such as missing log messages). */ - myWaitStatus = *((volatile int *) &MyProc->waitStatus); + myWaitStatus = *((volatile ProcWaitStatus *) &MyProc->waitStatus); /* * If we are not deadlocked, but are waiting on an autovacuum-induced @@ -1481,24 +1481,24 @@ ProcSleep(LOCALLOCK *locallock, LockMethod lockMethodTable) lockHoldersNum, lock_holders_sbuf.data, lock_waiters_sbuf.data)))); } - if (myWaitStatus == STATUS_WAITING) + if (myWaitStatus == PROC_WAIT_STATUS_WAITING) ereport(LOG, (errmsg("process %d still waiting for %s on %s after %ld.%03d ms", MyProcPid, modename, buf.data, msecs, usecs), (errdetail_log_plural("Process holding the lock: %s. Wait queue: %s.", "Processes holding the lock: %s. Wait queue: %s.", lockHoldersNum, lock_holders_sbuf.data, lock_waiters_sbuf.data)))); - else if (myWaitStatus == STATUS_OK) + else if (myWaitStatus == PROC_WAIT_STATUS_OK) ereport(LOG, (errmsg("process %d acquired %s on %s after %ld.%03d ms", MyProcPid, modename, buf.data, msecs, usecs))); else { - Assert(myWaitStatus == STATUS_ERROR); + Assert(myWaitStatus == PROC_WAIT_STATUS_ERROR); /* * Currently, the deadlock checker always kicks its own - * process, which means that we'll only see STATUS_ERROR when + * process, which means that we'll only see PROC_WAIT_STATUS_ERROR when * deadlock_state == DS_HARD_DEADLOCK, and there's no need to * print redundant messages. But for completeness and * future-proofing, print a message if it looks like someone @@ -1523,7 +1523,7 @@ ProcSleep(LOCALLOCK *locallock, LockMethod lockMethodTable) pfree(lock_holders_sbuf.data); pfree(lock_waiters_sbuf.data); } - } while (myWaitStatus == STATUS_WAITING); + } while (myWaitStatus == PROC_WAIT_STATUS_WAITING); /* * Disable the timers, if they are still running. As in LockErrorCleanup, @@ -1562,7 +1562,7 @@ ProcSleep(LOCALLOCK *locallock, LockMethod lockMethodTable) /* * If we got the lock, be sure to remember it in the locallock table. */ - if (MyProc->waitStatus == STATUS_OK) + if (MyProc->waitStatus == PROC_WAIT_STATUS_OK) GrantAwaitedLock(); /* @@ -1584,10 +1584,10 @@ ProcSleep(LOCALLOCK *locallock, LockMethod lockMethodTable) * XXX: presently, this code is only used for the "success" case, and only * works correctly for that case. To clean up in failure case, would need * to twiddle the lock's request counts too --- see RemoveFromWaitQueue. - * Hence, in practice the waitStatus parameter must be STATUS_OK. + * Hence, in practice the waitStatus parameter must be PROC_WAIT_STATUS_OK. */ PGPROC * -ProcWakeup(PGPROC *proc, int waitStatus) +ProcWakeup(PGPROC *proc, ProcWaitStatus waitStatus) { PGPROC *retProc; @@ -1595,7 +1595,7 @@ ProcWakeup(PGPROC *proc, int waitStatus) if (proc->links.prev == NULL || proc->links.next == NULL) return NULL; - Assert(proc->waitStatus == STATUS_WAITING); + Assert(proc->waitStatus == PROC_WAIT_STATUS_WAITING); /* Save next process before we zap the list link */ retProc = (PGPROC *) proc->links.next; @@ -1651,7 +1651,7 @@ ProcLockWakeup(LockMethod lockMethodTable, LOCK *lock) { /* OK to waken */ GrantLock(lock, proc->waitProcLock, lockmode); - proc = ProcWakeup(proc, STATUS_OK); + proc = ProcWakeup(proc, PROC_WAIT_STATUS_OK); /* * ProcWakeup removes proc from the lock's waiting process queue @@ -1731,7 +1731,7 @@ CheckDeadLock(void) * preserve the flexibility to kill some other transaction than the * one detecting the deadlock.) * - * RemoveFromWaitQueue sets MyProc->waitStatus to STATUS_ERROR, so + * RemoveFromWaitQueue sets MyProc->waitStatus to PROC_WAIT_STATUS_ERROR, so * ProcSleep will report an error after we return from the signal * handler. */ diff --git a/src/include/c.h b/src/include/c.h index eddeb36ca1..f4b34cf243 100644 --- a/src/include/c.h +++ b/src/include/c.h @@ -1120,7 +1120,6 @@ typedef union PGAlignedXLogBlock #define STATUS_OK (0) #define STATUS_ERROR (-1) #define STATUS_EOF (-2) -#define STATUS_WAITING (2) /* * gettext support diff --git a/src/include/storage/proc.h b/src/include/storage/proc.h index 281e1db725..180c2259d3 100644 --- a/src/include/storage/proc.h +++ b/src/include/storage/proc.h @@ -76,6 +76,13 @@ struct XidCache */ #define INVALID_PGPROCNO PG_INT32_MAX +typedef enum +{ + PROC_WAIT_STATUS_OK, + PROC_WAIT_STATUS_WAITING, + PROC_WAIT_STATUS_ERROR, +} ProcWaitStatus; + /* * Each backend has a PGPROC struct in shared memory. There is also a list of * currently-unused PGPROC structs that will be reallocated to new backends. @@ -99,7 +106,7 @@ struct PGPROC PGPROC **procgloballist; /* procglobal list that owns this PGPROC */ PGSemaphore sem; /* ONE semaphore to sleep on */ - int waitStatus; /* STATUS_WAITING, STATUS_OK or STATUS_ERROR */ + ProcWaitStatus waitStatus; Latch procLatch; /* generic latch for process */ @@ -317,8 +324,8 @@ extern bool HaveNFreeProcs(int n); extern void ProcReleaseLocks(bool isCommit); extern void ProcQueueInit(PROC_QUEUE *queue); -extern int ProcSleep(LOCALLOCK *locallock, LockMethod lockMethodTable); -extern PGPROC *ProcWakeup(PGPROC *proc, int waitStatus); +extern ProcWaitStatus ProcSleep(LOCALLOCK *locallock, LockMethod lockMethodTable); +extern PGPROC *ProcWakeup(PGPROC *proc, ProcWaitStatus waitStatus); extern void ProcLockWakeup(LockMethod lockMethodTable, LOCK *lock); extern void CheckDeadLockAlert(void); extern bool IsWaitingForLock(void); -- 2.24.1