From: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
To: pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: remove some STATUS_* symbols
Date: Sun, 29 Dec 2019 11:33:34 +0100
Message-ID: <a6f91ead-0ce4-2a34-062b-7ab9813ea308@2ndquadrant.com> (raw)
I have found the collection of STATUS_* defines in c.h a bit curious.
There used to be a lot more even that have been removed over time.
Currently, STATUS_FOUND and STATUS_WAITING are only used in one group of
functions each, so perhaps it would make more sense to remove these from
the global namespace and make them a local concern.
Attached are two patches to remove these two symbols. STATUS_FOUND can
be replaced by a simple bool. STATUS_WAITING is replaced by a separate
enum.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From f575da0fedc920003afb4655f3503cc91949cc55 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Sun, 29 Dec 2019 09:09:20 +0100
Subject: [PATCH 1/4] Remove STATUS_FOUND
Replace the solitary use with a bool.
---
src/backend/storage/lmgr/lock.c | 24 +++++++++++-------------
src/backend/storage/lmgr/proc.c | 12 ++++--------
src/include/c.h | 1 -
src/include/storage/lock.h | 2 +-
4 files changed, 16 insertions(+), 23 deletions(-)
diff --git a/src/backend/storage/lmgr/lock.c b/src/backend/storage/lmgr/lock.cindex 9089733ecc..e2c728a97d 100644--- a/src/backend/storage/lmgr/lock.c+++ b/src/backend/storage/lmgr/lock.c@@ -746,7 +746,7 @@ LockAcquireExtended(const LOCKTAG *locktag,
ResourceOwner owner;
uint32 hashcode;
LWLock *partitionLock;
- int status;+ bool found_conflict;
bool log_lock = false;
if (lockmethodid <= 0 || lockmethodid >= lengthof(LockMethods))
@@ -979,12 +979,12 @@ LockAcquireExtended(const LOCKTAG *locktag,
* (That's last because most complex check.)
*/
if (lockMethodTable->conflictTab[lockmode] & lock->waitMask)
- status = STATUS_FOUND;+ found_conflict = true;
else
- status = LockCheckConflicts(lockMethodTable, lockmode,+ found_conflict = LockCheckConflicts(lockMethodTable, lockmode,
lock, proclock);
- if (status == STATUS_OK)+ if (!found_conflict)
{
/* No conflict with held or previously requested locks */
GrantLock(lock, proclock, lockmode);
@@ -992,8 +992,6 @@ LockAcquireExtended(const LOCKTAG *locktag,
}
else
{
- Assert(status == STATUS_FOUND);-
/*
* We can't acquire the lock immediately. If caller specified no
* blocking, remove useless table entries and return
@@ -1330,7 +1328,7 @@ RemoveLocalLock(LOCALLOCK *locallock)
* LockCheckConflicts -- test whether requested lock conflicts
* with those already granted
*
- * Returns STATUS_FOUND if conflict, STATUS_OK if no conflict.+ * Returns true if conflict, false if no conflict.
*
* NOTES:
* Here's what makes this complicated: one process's locks don't
@@ -1340,7 +1338,7 @@ RemoveLocalLock(LOCALLOCK *locallock)
* the same group. So, we must subtract off these locks when determining
* whether the requested new lock conflicts with those already held.
*/
-int+bool
LockCheckConflicts(LockMethod lockMethodTable,
LOCKMODE lockmode,
LOCK *lock,
@@ -1367,7 +1365,7 @@ LockCheckConflicts(LockMethod lockMethodTable,
if (!(conflictMask & lock->grantMask))
{
PROCLOCK_PRINT("LockCheckConflicts: no conflict", proclock);
- return STATUS_OK;+ return false;
}
/*
@@ -1393,7 +1391,7 @@ LockCheckConflicts(LockMethod lockMethodTable,
if (totalConflictsRemaining == 0)
{
PROCLOCK_PRINT("LockCheckConflicts: resolved (simple)", proclock);
- return STATUS_OK;+ return false;
}
/* If no group locking, it's definitely a conflict. */
@@ -1402,7 +1400,7 @@ LockCheckConflicts(LockMethod lockMethodTable,
Assert(proclock->tag.myProc == MyProc);
PROCLOCK_PRINT("LockCheckConflicts: conflicting (simple)",
proclock);
- return STATUS_FOUND;+ return true;
}
/*
@@ -1439,7 +1437,7 @@ LockCheckConflicts(LockMethod lockMethodTable,
{
PROCLOCK_PRINT("LockCheckConflicts: resolved (group)",
proclock);
- return STATUS_OK;+ return false;
}
}
otherproclock = (PROCLOCK *)
@@ -1449,7 +1447,7 @@ LockCheckConflicts(LockMethod lockMethodTable,
/* Nope, it's a real conflict. */
PROCLOCK_PRINT("LockCheckConflicts: conflicting (group)", proclock);
- return STATUS_FOUND;+ return true;
}
/*
diff --git a/src/backend/storage/lmgr/proc.c b/src/backend/storage/lmgr/proc.cindex fff0628e58..dbb541757f 100644--- a/src/backend/storage/lmgr/proc.c+++ b/src/backend/storage/lmgr/proc.c@@ -1149,10 +1149,8 @@ ProcSleep(LOCALLOCK *locallock, LockMethod lockMethodTable)
}
/* I must go before this waiter. Check special case. */
if ((lockMethodTable->conflictTab[lockmode] & aheadRequests) == 0 &&
- LockCheckConflicts(lockMethodTable,- lockmode,- lock,- proclock) == STATUS_OK)+ !LockCheckConflicts(lockMethodTable, lockmode, lock,+ proclock))
{
/* Skip the wait and just grant myself the lock. */
GrantLock(lock, proclock, lockmode);
@@ -1648,10 +1646,8 @@ ProcLockWakeup(LockMethod lockMethodTable, LOCK *lock)
* (b) doesn't conflict with already-held locks.
*/
if ((lockMethodTable->conflictTab[lockmode] & aheadRequests) == 0 &&
- LockCheckConflicts(lockMethodTable,- lockmode,- lock,- proc->waitProcLock) == STATUS_OK)+ !LockCheckConflicts(lockMethodTable, lockmode, lock,+ proc->waitProcLock))
{
/* OK to waken */
GrantLock(lock, proc->waitProcLock, lockmode);
diff --git a/src/include/c.h b/src/include/c.hindex 00e41ac546..eddeb36ca1 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_FOUND (1)
#define STATUS_WAITING (2)
/*
diff --git a/src/include/storage/lock.h b/src/include/storage/lock.hindex e8b50fe094..7978fcf216 100644--- a/src/include/storage/lock.h+++ b/src/include/storage/lock.h@@ -550,7 +550,7 @@ extern VirtualTransactionId *GetLockConflicts(const LOCKTAG *locktag,
LOCKMODE lockmode, int *countp);
extern void AtPrepare_Locks(void);
extern void PostPrepare_Locks(TransactionId xid);
-extern int LockCheckConflicts(LockMethod lockMethodTable,+extern bool LockCheckConflicts(LockMethod lockMethodTable,
LOCKMODE lockmode,
LOCK *lock, PROCLOCK *proclock);
extern void GrantLock(LOCK *lock, PROCLOCK *proclock, LOCKMODE lockmode);
--
2.24.1
From 049fa9519d14b3b01bffbd219ebbc8340987f90e Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
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.cindex 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.cindex 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.cindex 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.hindex 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.hindex 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
Attachments:
[text/plain] 0001-Remove-STATUS_FOUND.patch (5.7K, ../a6f91ead-0ce4-2a34-062b-7ab9813ea308@2ndquadrant.com/2-0001-Remove-STATUS_FOUND.patch)
download | inline diff:
From f575da0fedc920003afb4655f3503cc91949cc55 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Sun, 29 Dec 2019 09:09:20 +0100
Subject: [PATCH 1/4] Remove STATUS_FOUND
Replace the solitary use with a bool.
---
src/backend/storage/lmgr/lock.c | 24 +++++++++++-------------
src/backend/storage/lmgr/proc.c | 12 ++++--------
src/include/c.h | 1 -
src/include/storage/lock.h | 2 +-
4 files changed, 16 insertions(+), 23 deletions(-)
diff --git a/src/backend/storage/lmgr/lock.c b/src/backend/storage/lmgr/lock.cindex 9089733ecc..e2c728a97d 100644--- a/src/backend/storage/lmgr/lock.c+++ b/src/backend/storage/lmgr/lock.c@@ -746,7 +746,7 @@ LockAcquireExtended(const LOCKTAG *locktag,
ResourceOwner owner;
uint32 hashcode;
LWLock *partitionLock;
- int status;+ bool found_conflict;
bool log_lock = false;
if (lockmethodid <= 0 || lockmethodid >= lengthof(LockMethods))
@@ -979,12 +979,12 @@ LockAcquireExtended(const LOCKTAG *locktag,
* (That's last because most complex check.)
*/
if (lockMethodTable->conflictTab[lockmode] & lock->waitMask)
- status = STATUS_FOUND;+ found_conflict = true;
else
- status = LockCheckConflicts(lockMethodTable, lockmode,+ found_conflict = LockCheckConflicts(lockMethodTable, lockmode,
lock, proclock);
- if (status == STATUS_OK)+ if (!found_conflict)
{
/* No conflict with held or previously requested locks */
GrantLock(lock, proclock, lockmode);
@@ -992,8 +992,6 @@ LockAcquireExtended(const LOCKTAG *locktag,
}
else
{
- Assert(status == STATUS_FOUND);-
/*
* We can't acquire the lock immediately. If caller specified no
* blocking, remove useless table entries and return
@@ -1330,7 +1328,7 @@ RemoveLocalLock(LOCALLOCK *locallock)
* LockCheckConflicts -- test whether requested lock conflicts
* with those already granted
*
- * Returns STATUS_FOUND if conflict, STATUS_OK if no conflict.+ * Returns true if conflict, false if no conflict.
*
* NOTES:
* Here's what makes this complicated: one process's locks don't
@@ -1340,7 +1338,7 @@ RemoveLocalLock(LOCALLOCK *locallock)
* the same group. So, we must subtract off these locks when determining
* whether the requested new lock conflicts with those already held.
*/
-int+bool
LockCheckConflicts(LockMethod lockMethodTable,
LOCKMODE lockmode,
LOCK *lock,
@@ -1367,7 +1365,7 @@ LockCheckConflicts(LockMethod lockMethodTable,
if (!(conflictMask & lock->grantMask))
{
PROCLOCK_PRINT("LockCheckConflicts: no conflict", proclock);
- return STATUS_OK;+ return false;
}
/*
@@ -1393,7 +1391,7 @@ LockCheckConflicts(LockMethod lockMethodTable,
if (totalConflictsRemaining == 0)
{
PROCLOCK_PRINT("LockCheckConflicts: resolved (simple)", proclock);
- return STATUS_OK;+ return false;
}
/* If no group locking, it's definitely a conflict. */
@@ -1402,7 +1400,7 @@ LockCheckConflicts(LockMethod lockMethodTable,
Assert(proclock->tag.myProc == MyProc);
PROCLOCK_PRINT("LockCheckConflicts: conflicting (simple)",
proclock);
- return STATUS_FOUND;+ return true;
}
/*
@@ -1439,7 +1437,7 @@ LockCheckConflicts(LockMethod lockMethodTable,
{
PROCLOCK_PRINT("LockCheckConflicts: resolved (group)",
proclock);
- return STATUS_OK;+ return false;
}
}
otherproclock = (PROCLOCK *)
@@ -1449,7 +1447,7 @@ LockCheckConflicts(LockMethod lockMethodTable,
/* Nope, it's a real conflict. */
PROCLOCK_PRINT("LockCheckConflicts: conflicting (group)", proclock);
- return STATUS_FOUND;+ return true;
}
/*
diff --git a/src/backend/storage/lmgr/proc.c b/src/backend/storage/lmgr/proc.cindex fff0628e58..dbb541757f 100644--- a/src/backend/storage/lmgr/proc.c+++ b/src/backend/storage/lmgr/proc.c@@ -1149,10 +1149,8 @@ ProcSleep(LOCALLOCK *locallock, LockMethod lockMethodTable)
}
/* I must go before this waiter. Check special case. */
if ((lockMethodTable->conflictTab[lockmode] & aheadRequests) == 0 &&
- LockCheckConflicts(lockMethodTable,- lockmode,- lock,- proclock) == STATUS_OK)+ !LockCheckConflicts(lockMethodTable, lockmode, lock,+ proclock))
{
/* Skip the wait and just grant myself the lock. */
GrantLock(lock, proclock, lockmode);
@@ -1648,10 +1646,8 @@ ProcLockWakeup(LockMethod lockMethodTable, LOCK *lock)
* (b) doesn't conflict with already-held locks.
*/
if ((lockMethodTable->conflictTab[lockmode] & aheadRequests) == 0 &&
- LockCheckConflicts(lockMethodTable,- lockmode,- lock,- proc->waitProcLock) == STATUS_OK)+ !LockCheckConflicts(lockMethodTable, lockmode, lock,+ proc->waitProcLock))
{
/* OK to waken */
GrantLock(lock, proc->waitProcLock, lockmode);
diff --git a/src/include/c.h b/src/include/c.hindex 00e41ac546..eddeb36ca1 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_FOUND (1)
#define STATUS_WAITING (2)
/*
diff --git a/src/include/storage/lock.h b/src/include/storage/lock.hindex e8b50fe094..7978fcf216 100644--- a/src/include/storage/lock.h+++ b/src/include/storage/lock.h@@ -550,7 +550,7 @@ extern VirtualTransactionId *GetLockConflicts(const LOCKTAG *locktag,
LOCKMODE lockmode, int *countp);
extern void AtPrepare_Locks(void);
extern void PostPrepare_Locks(TransactionId xid);
-extern int LockCheckConflicts(LockMethod lockMethodTable,+extern bool LockCheckConflicts(LockMethod lockMethodTable,
LOCKMODE lockmode,
LOCK *lock, PROCLOCK *proclock);
extern void GrantLock(LOCK *lock, PROCLOCK *proclock, LOCKMODE lockmode);
--
2.24.1
[text/plain] 0002-Remove-STATUS_WAITING.patch (12.1K, ../a6f91ead-0ce4-2a34-062b-7ab9813ea308@2ndquadrant.com/3-0002-Remove-STATUS_WAITING.patch)
download | inline diff:
From 049fa9519d14b3b01bffbd219ebbc8340987f90e Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
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.cindex 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.cindex 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.cindex 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.hindex 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.hindex 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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: peter.eisentraut@2ndquadrant.com
Subject: Re: remove some STATUS_* symbols
In-Reply-To: <a6f91ead-0ce4-2a34-062b-7ab9813ea308@2ndquadrant.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox