Hi Soumyadeep, On Sat, Jul 24, 2021 at 5:26 PM Soumyadeep Chakraborty <soumyadeep2...@gmail.com> wrote: > On Tue, Jul 20, 2021 at 10:40 PM Thomas Munro <thomas.mu...@gmail.com> wrote: > > I wonder why we need this member anyway, when you can compute it from > > the address... #define GetPGProcNumber(p) ((p) - ProcGlobal->allProcs) > > or something like that? Kinda wonder why we don't use > > GetPGProcByNumber() in more places instead of open-coding access to > > ProcGlobal->allProcs, too... > > I tried this out. See attached v4 of your patch with these changes.
I like it. I've moved these changes to a separate patch, 0002, for separate commit. I tweaked a couple of comments (it's not a position in the "procarray", well it's a position stored in the procarray, but that's confusing; I also found a stray comment about leader->pgprocno that is obsoleted by this change). Does anyone have objections to this? I was going to commit the earlier change this morning, but then I read [1]. New idea. Instead of adding pgprocno to SERIALIZABLEXACT (which we should really be trying to shrink, not grow), let's look it up by vxid->backendId. I didn't consider that before, because I was trying not to get tangled up with BackendIds for various reasons, not least that that's yet another lock + O(n) search. It seems likely that getting from vxid to latch will be less clumsy in the near future. That refactoring and harmonising of backend identifiers seems like a great idea to me. Here's a version that anticipates that, using vxid->backendId to wake a sleeping SERIALIZABLE READ ONLY DEFERRABLE backend, without having to add a new member to the struct. > A session ID seems a bit heavy just to indicate whether a backend has > exited. Yeah. A Greenplum-like session ID might eventually be necessary in a world where sessions are divorced from processes and handled by a pool of worker threads, though. /me gazes towards the horizon [1] https://www.postgresql.org/message-id/flat/20210802164124.ufo5buo4apl6yuvs%40alap3.anarazel.de
From b284d8f29efc1c16c3aa75b64d9a940bcb74872c Mon Sep 17 00:00:00 2001 From: Thomas Munro <tmu...@postgresql.org> Date: Tue, 3 Aug 2021 10:02:15 +1200 Subject: [PATCH v5 1/2] Optimize ProcSendSignal(). Instead of referring to target backends by pid, use pgprocno. This means that we don't have to scan the ProcArray, and we can drop some special case code for dealing with the startup process. In the case of buffer pin waits, we switch to storing the pgprocno of the waiter. In the case of SERIALIZABLE READ ONLY DEFERRABLE waits, we derive the pgprocno from the vxid (though that's not yet as efficient as it could be). Reviewed-by: Soumyadeep Chakraborty <soumyadeep2...@gmail.com> Reviewed-by: Ashwin Agrawal <ashwins...@gmail.com> Discussion: https://postgr.es/m/CA%2BhUKGLYRyDaneEwz5Uya_OgFLMx5BgJfkQSD%3Dq9HmwsfRRb-w%40mail.gmail.com --- src/backend/access/transam/xlog.c | 3 -- src/backend/storage/buffer/buf_init.c | 3 +- src/backend/storage/buffer/bufmgr.c | 10 +++--- src/backend/storage/lmgr/predicate.c | 8 ++++- src/backend/storage/lmgr/proc.c | 49 ++------------------------- src/include/storage/buf_internals.h | 2 +- src/include/storage/proc.h | 6 +--- 7 files changed, 19 insertions(+), 62 deletions(-) diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index f84c0bb01e..1574362724 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -7308,9 +7308,6 @@ StartupXLOG(void) /* Also ensure XLogReceiptTime has a sane value */ XLogReceiptTime = GetCurrentTimestamp(); - /* Allow ProcSendSignal() to find us, for buffer pin wakeups. */ - PublishStartupProcessInformation(); - /* * Let postmaster know we've started redo now, so that it can launch * the archiver if necessary. diff --git a/src/backend/storage/buffer/buf_init.c b/src/backend/storage/buffer/buf_init.c index a299be1043..b9a83c0e9b 100644 --- a/src/backend/storage/buffer/buf_init.c +++ b/src/backend/storage/buffer/buf_init.c @@ -16,6 +16,7 @@ #include "storage/buf_internals.h" #include "storage/bufmgr.h" +#include "storage/proc.h" BufferDescPadded *BufferDescriptors; char *BufferBlocks; @@ -118,7 +119,7 @@ InitBufferPool(void) CLEAR_BUFFERTAG(buf->tag); pg_atomic_init_u32(&buf->state, 0); - buf->wait_backend_pid = 0; + buf->wait_backend_pgprocno = INVALID_PGPROCNO; buf->buf_id = i; diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c index 33d99f604a..fb86c8706c 100644 --- a/src/backend/storage/buffer/bufmgr.c +++ b/src/backend/storage/buffer/bufmgr.c @@ -1890,11 +1890,11 @@ UnpinBuffer(BufferDesc *buf, bool fixOwner) BUF_STATE_GET_REFCOUNT(buf_state) == 1) { /* we just released the last pin other than the waiter's */ - int wait_backend_pid = buf->wait_backend_pid; + int wait_backend_pgprocno = buf->wait_backend_pgprocno; buf_state &= ~BM_PIN_COUNT_WAITER; UnlockBufHdr(buf, buf_state); - ProcSendSignal(wait_backend_pid); + ProcSendSignal(wait_backend_pgprocno); } else UnlockBufHdr(buf, buf_state); @@ -3995,7 +3995,7 @@ UnlockBuffers(void) * got a cancel/die interrupt before getting the signal. */ if ((buf_state & BM_PIN_COUNT_WAITER) != 0 && - buf->wait_backend_pid == MyProcPid) + buf->wait_backend_pgprocno == MyProc->pgprocno) buf_state &= ~BM_PIN_COUNT_WAITER; UnlockBufHdr(buf, buf_state); @@ -4131,7 +4131,7 @@ LockBufferForCleanup(Buffer buffer) LockBuffer(buffer, BUFFER_LOCK_UNLOCK); elog(ERROR, "multiple backends attempting to wait for pincount 1"); } - bufHdr->wait_backend_pid = MyProcPid; + bufHdr->wait_backend_pgprocno = MyProc->pgprocno; PinCountWaitBuf = bufHdr; buf_state |= BM_PIN_COUNT_WAITER; UnlockBufHdr(bufHdr, buf_state); @@ -4202,7 +4202,7 @@ LockBufferForCleanup(Buffer buffer) */ buf_state = LockBufHdr(bufHdr); if ((buf_state & BM_PIN_COUNT_WAITER) != 0 && - bufHdr->wait_backend_pid == MyProcPid) + bufHdr->wait_backend_pgprocno == MyProc->pgprocno) buf_state &= ~BM_PIN_COUNT_WAITER; UnlockBufHdr(bufHdr, buf_state); diff --git a/src/backend/storage/lmgr/predicate.c b/src/backend/storage/lmgr/predicate.c index 56267bdc3c..fbc0caca5c 100644 --- a/src/backend/storage/lmgr/predicate.c +++ b/src/backend/storage/lmgr/predicate.c @@ -207,6 +207,7 @@ #include "storage/predicate_internals.h" #include "storage/proc.h" #include "storage/procarray.h" +#include "storage/sinvaladt.h" #include "utils/rel.h" #include "utils/snapmgr.h" @@ -3669,7 +3670,12 @@ ReleasePredicateLocks(bool isCommit, bool isReadOnlySafe) */ if (SxactIsDeferrableWaiting(roXact) && (SxactIsROUnsafe(roXact) || SxactIsROSafe(roXact))) - ProcSendSignal(roXact->pid); + { + PGPROC *proc; + + proc = BackendIdGetProc(roXact->vxid.backendId); + ProcSendSignal(proc->pgprocno); + } possibleUnsafeConflict = nextConflict; } diff --git a/src/backend/storage/lmgr/proc.c b/src/backend/storage/lmgr/proc.c index b7d9da0aa9..7f2111c6e0 100644 --- a/src/backend/storage/lmgr/proc.c +++ b/src/backend/storage/lmgr/proc.c @@ -177,8 +177,6 @@ InitProcGlobal(void) ProcGlobal->autovacFreeProcs = NULL; ProcGlobal->bgworkerFreeProcs = NULL; ProcGlobal->walsenderFreeProcs = NULL; - ProcGlobal->startupProc = NULL; - ProcGlobal->startupProcPid = 0; ProcGlobal->startupBufferPinWaitBufId = -1; ProcGlobal->walwriterLatch = NULL; ProcGlobal->checkpointerLatch = NULL; @@ -624,21 +622,6 @@ InitAuxiliaryProcess(void) on_shmem_exit(AuxiliaryProcKill, Int32GetDatum(proctype)); } -/* - * Record the PID and PGPROC structures for the Startup process, for use in - * ProcSendSignal(). See comments there for further explanation. - */ -void -PublishStartupProcessInformation(void) -{ - SpinLockAcquire(ProcStructLock); - - ProcGlobal->startupProc = MyProc; - ProcGlobal->startupProcPid = MyProcPid; - - SpinLockRelease(ProcStructLock); -} - /* * Used from bufmgr to share the value of the buffer that Startup waits on, * or to reset the value to "not waiting" (-1). This allows processing @@ -1903,38 +1886,12 @@ ProcWaitForSignal(uint32 wait_event_info) } /* - * ProcSendSignal - send a signal to a backend identified by PID + * ProcSendSignal - send a signal to a backend identified by pgprocno */ void -ProcSendSignal(int pid) +ProcSendSignal(int pgprocno) { - PGPROC *proc = NULL; - - if (RecoveryInProgress()) - { - SpinLockAcquire(ProcStructLock); - - /* - * Check to see whether it is the Startup process we wish to signal. - * This call is made by the buffer manager when it wishes to wake up a - * process that has been waiting for a pin in so it can obtain a - * cleanup lock using LockBufferForCleanup(). Startup is not a normal - * backend, so BackendPidGetProc() will not return any pid at all. So - * we remember the information for this special case. - */ - if (pid == ProcGlobal->startupProcPid) - proc = ProcGlobal->startupProc; - - SpinLockRelease(ProcStructLock); - } - - if (proc == NULL) - proc = BackendPidGetProc(pid); - - if (proc != NULL) - { - SetLatch(&proc->procLatch); - } + SetLatch(&ProcGlobal->allProcs[pgprocno].procLatch); } /* diff --git a/src/include/storage/buf_internals.h b/src/include/storage/buf_internals.h index 33fcaf5c9a..9b634616e4 100644 --- a/src/include/storage/buf_internals.h +++ b/src/include/storage/buf_internals.h @@ -187,7 +187,7 @@ typedef struct BufferDesc /* state of the tag, containing flags, refcount and usagecount */ pg_atomic_uint32 state; - int wait_backend_pid; /* backend PID of pin-count waiter */ + int wait_backend_pgprocno; /* backend of pin-count waiter */ int freeNext; /* link in freelist chain */ LWLock content_lock; /* to lock access to buffer contents */ } BufferDesc; diff --git a/src/include/storage/proc.h b/src/include/storage/proc.h index be67d8a861..7c1e2efe30 100644 --- a/src/include/storage/proc.h +++ b/src/include/storage/proc.h @@ -352,9 +352,6 @@ typedef struct PROC_HDR Latch *checkpointerLatch; /* Current shared estimate of appropriate spins_per_delay value */ int spins_per_delay; - /* The proc of the Startup process, since not in ProcArray */ - PGPROC *startupProc; - int startupProcPid; /* Buffer id of the buffer that Startup process waits for pin on, or -1 */ int startupBufferPinWaitBufId; } PROC_HDR; @@ -395,7 +392,6 @@ extern void InitProcess(void); extern void InitProcessPhase2(void); extern void InitAuxiliaryProcess(void); -extern void PublishStartupProcessInformation(void); extern void SetStartupBufferPinWaitBufId(int bufid); extern int GetStartupBufferPinWaitBufId(void); @@ -411,7 +407,7 @@ extern bool IsWaitingForLock(void); extern void LockErrorCleanup(void); extern void ProcWaitForSignal(uint32 wait_event_info); -extern void ProcSendSignal(int pid); +extern void ProcSendSignal(int pgprocno); extern PGPROC *AuxiliaryPidGetProc(int pid); -- 2.30.2
From 562657ea3f7be124a6c6b6d1e7450da2431a54a0 Mon Sep 17 00:00:00 2001 From: Thomas Munro <thomas.mu...@gmail.com> Date: Thu, 11 Mar 2021 23:09:11 +1300 Subject: [PATCH v5 2/2] Remove PGPROC's redundant pgprocno member. It's derivable with pointer arithmetic. Author: Soumyadeep Chakraborty <soumyadeep2...@gmail.com> Discussion: https://postgr.es/m/CA%2BhUKGLYRyDaneEwz5Uya_OgFLMx5BgJfkQSD%3Dq9HmwsfRRb-w%40mail.gmail.com --- src/backend/access/transam/clog.c | 2 +- src/backend/access/transam/twophase.c | 3 +-- src/backend/access/transam/xlog.c | 2 +- src/backend/postmaster/bgwriter.c | 2 +- src/backend/postmaster/pgarch.c | 2 +- src/backend/storage/buffer/bufmgr.c | 6 +++--- src/backend/storage/ipc/procarray.c | 6 +++--- src/backend/storage/lmgr/condition_variable.c | 12 ++++++------ src/backend/storage/lmgr/lwlock.c | 6 +++--- src/backend/storage/lmgr/predicate.c | 2 +- src/backend/storage/lmgr/proc.c | 8 +++----- src/backend/utils/adt/lockfuncs.c | 1 + src/include/storage/lock.h | 2 +- src/include/storage/proc.h | 6 +++++- 14 files changed, 31 insertions(+), 29 deletions(-) diff --git a/src/backend/access/transam/clog.c b/src/backend/access/transam/clog.c index 3ea16a270a..d0557f6c55 100644 --- a/src/backend/access/transam/clog.c +++ b/src/backend/access/transam/clog.c @@ -464,7 +464,7 @@ TransactionGroupUpdateXidStatus(TransactionId xid, XidStatus status, if (pg_atomic_compare_exchange_u32(&procglobal->clogGroupFirst, &nextidx, - (uint32) proc->pgprocno)) + (uint32) GetPGProcNumber(proc))) break; } diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c index 6d3efb49a4..0fe8a59cf0 100644 --- a/src/backend/access/transam/twophase.c +++ b/src/backend/access/transam/twophase.c @@ -279,7 +279,7 @@ TwoPhaseShmemInit(void) TwoPhaseState->freeGXacts = &gxacts[i]; /* associate it with a PGPROC assigned by InitProcGlobal */ - gxacts[i].pgprocno = PreparedXactProcs[i].pgprocno; + gxacts[i].pgprocno = GetPGProcNumber(&PreparedXactProcs[i]); /* * Assign a unique ID for each dummy proc, so that the range of @@ -456,7 +456,6 @@ MarkAsPreparingGuts(GlobalTransaction gxact, TransactionId xid, const char *gid, /* Initialize the PGPROC entry */ MemSet(proc, 0, sizeof(PGPROC)); - proc->pgprocno = gxact->pgprocno; SHMQueueElemInit(&(proc->links)); proc->waitStatus = PROC_WAIT_STATUS_OK; /* We set up the gxact's VXID as InvalidBackendId/XID */ diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index 1574362724..576ba061d8 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -1666,7 +1666,7 @@ WALInsertLockAcquire(void) static int lockToTry = -1; if (lockToTry == -1) - lockToTry = MyProc->pgprocno % NUM_XLOGINSERT_LOCKS; + lockToTry = GetPGProcNumber(MyProc) % NUM_XLOGINSERT_LOCKS; MyLockNo = lockToTry; /* diff --git a/src/backend/postmaster/bgwriter.c b/src/backend/postmaster/bgwriter.c index 5584f4bc24..8c812e906d 100644 --- a/src/backend/postmaster/bgwriter.c +++ b/src/backend/postmaster/bgwriter.c @@ -333,7 +333,7 @@ BackgroundWriterMain(void) if (rc == WL_TIMEOUT && can_hibernate && prev_hibernate) { /* Ask for notification at next buffer allocation */ - StrategyNotifyBgWriter(MyProc->pgprocno); + StrategyNotifyBgWriter(GetPGProcNumber(MyProc)); /* Sleep ... */ (void) WaitLatch(MyLatch, WL_LATCH_SET | WL_TIMEOUT | WL_EXIT_ON_PM_DEATH, diff --git a/src/backend/postmaster/pgarch.c b/src/backend/postmaster/pgarch.c index 74a7d7c4d0..68de7ee3b8 100644 --- a/src/backend/postmaster/pgarch.c +++ b/src/backend/postmaster/pgarch.c @@ -196,7 +196,7 @@ PgArchiverMain(void) * Advertise our pgprocno so that backends can use our latch to wake us up * while we're sleeping. */ - PgArch->pgprocno = MyProc->pgprocno; + PgArch->pgprocno = GetPGProcNumber(MyProc); pgarch_MainLoop(); diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c index fb86c8706c..ad95e2f938 100644 --- a/src/backend/storage/buffer/bufmgr.c +++ b/src/backend/storage/buffer/bufmgr.c @@ -3995,7 +3995,7 @@ UnlockBuffers(void) * got a cancel/die interrupt before getting the signal. */ if ((buf_state & BM_PIN_COUNT_WAITER) != 0 && - buf->wait_backend_pgprocno == MyProc->pgprocno) + buf->wait_backend_pgprocno == GetPGProcNumber(MyProc)) buf_state &= ~BM_PIN_COUNT_WAITER; UnlockBufHdr(buf, buf_state); @@ -4131,7 +4131,7 @@ LockBufferForCleanup(Buffer buffer) LockBuffer(buffer, BUFFER_LOCK_UNLOCK); elog(ERROR, "multiple backends attempting to wait for pincount 1"); } - bufHdr->wait_backend_pgprocno = MyProc->pgprocno; + bufHdr->wait_backend_pgprocno = GetPGProcNumber(MyProc); PinCountWaitBuf = bufHdr; buf_state |= BM_PIN_COUNT_WAITER; UnlockBufHdr(bufHdr, buf_state); @@ -4202,7 +4202,7 @@ LockBufferForCleanup(Buffer buffer) */ buf_state = LockBufHdr(bufHdr); if ((buf_state & BM_PIN_COUNT_WAITER) != 0 && - bufHdr->wait_backend_pgprocno == MyProc->pgprocno) + bufHdr->wait_backend_pgprocno == GetPGProcNumber(MyProc)) buf_state &= ~BM_PIN_COUNT_WAITER; UnlockBufHdr(bufHdr, buf_state); diff --git a/src/backend/storage/ipc/procarray.c b/src/backend/storage/ipc/procarray.c index c7816fcfb3..a2f53a9ce9 100644 --- a/src/backend/storage/ipc/procarray.c +++ b/src/backend/storage/ipc/procarray.c @@ -492,7 +492,7 @@ ProcArrayAdd(PGPROC *proc) Assert(allProcs[procno].pgxactoff == index); /* If we have found our right position in the array, break */ - if (arrayP->pgprocnos[index] > proc->pgprocno) + if (arrayP->pgprocnos[index] > GetPGProcNumber(proc)) break; } @@ -510,7 +510,7 @@ ProcArrayAdd(PGPROC *proc) &ProcGlobal->statusFlags[index], movecount * sizeof(*ProcGlobal->statusFlags)); - arrayP->pgprocnos[index] = proc->pgprocno; + arrayP->pgprocnos[index] = GetPGProcNumber(proc); proc->pgxactoff = index; ProcGlobal->xids[index] = proc->xid; ProcGlobal->subxidStates[index] = proc->subxidStatus; @@ -789,7 +789,7 @@ ProcArrayGroupClearXid(PGPROC *proc, TransactionId latestXid) if (pg_atomic_compare_exchange_u32(&procglobal->procArrayGroupFirst, &nextidx, - (uint32) proc->pgprocno)) + (uint32) GetPGProcNumber(proc))) break; } diff --git a/src/backend/storage/lmgr/condition_variable.c b/src/backend/storage/lmgr/condition_variable.c index 80d70c154c..d0728d17bd 100644 --- a/src/backend/storage/lmgr/condition_variable.c +++ b/src/backend/storage/lmgr/condition_variable.c @@ -57,7 +57,7 @@ ConditionVariableInit(ConditionVariable *cv) void ConditionVariablePrepareToSleep(ConditionVariable *cv) { - int pgprocno = MyProc->pgprocno; + int pgprocno = GetPGProcNumber(MyProc); /* * If some other sleep is already prepared, cancel it; this is necessary @@ -181,10 +181,10 @@ ConditionVariableTimedSleep(ConditionVariable *cv, long timeout, * guarantee not to return spuriously, we'll avoid this obvious case. */ SpinLockAcquire(&cv->mutex); - if (!proclist_contains(&cv->wakeup, MyProc->pgprocno, cvWaitLink)) + if (!proclist_contains(&cv->wakeup, GetPGProcNumber(MyProc), cvWaitLink)) { done = true; - proclist_push_tail(&cv->wakeup, MyProc->pgprocno, cvWaitLink); + proclist_push_tail(&cv->wakeup, GetPGProcNumber(MyProc), cvWaitLink); } SpinLockRelease(&cv->mutex); @@ -234,8 +234,8 @@ ConditionVariableCancelSleep(void) return; SpinLockAcquire(&cv->mutex); - if (proclist_contains(&cv->wakeup, MyProc->pgprocno, cvWaitLink)) - proclist_delete(&cv->wakeup, MyProc->pgprocno, cvWaitLink); + if (proclist_contains(&cv->wakeup, GetPGProcNumber(MyProc), cvWaitLink)) + proclist_delete(&cv->wakeup, GetPGProcNumber(MyProc), cvWaitLink); else signaled = true; SpinLockRelease(&cv->mutex); @@ -285,7 +285,7 @@ ConditionVariableSignal(ConditionVariable *cv) void ConditionVariableBroadcast(ConditionVariable *cv) { - int pgprocno = MyProc->pgprocno; + int pgprocno = GetPGProcNumber(MyProc); PGPROC *proc = NULL; bool have_sentinel = false; diff --git a/src/backend/storage/lmgr/lwlock.c b/src/backend/storage/lmgr/lwlock.c index 862097352b..e3cef6700a 100644 --- a/src/backend/storage/lmgr/lwlock.c +++ b/src/backend/storage/lmgr/lwlock.c @@ -1080,9 +1080,9 @@ LWLockQueueSelf(LWLock *lock, LWLockMode mode) /* LW_WAIT_UNTIL_FREE waiters are always at the front of the queue */ if (mode == LW_WAIT_UNTIL_FREE) - proclist_push_head(&lock->waiters, MyProc->pgprocno, lwWaitLink); + proclist_push_head(&lock->waiters, GetPGProcNumber(MyProc), lwWaitLink); else - proclist_push_tail(&lock->waiters, MyProc->pgprocno, lwWaitLink); + proclist_push_tail(&lock->waiters, GetPGProcNumber(MyProc), lwWaitLink); /* Can release the mutex now */ LWLockWaitListUnlock(lock); @@ -1122,7 +1122,7 @@ LWLockDequeueSelf(LWLock *lock) */ proclist_foreach_modify(iter, &lock->waiters, lwWaitLink) { - if (iter.cur == MyProc->pgprocno) + if (iter.cur == GetPGProcNumber(MyProc)) { found = true; proclist_delete(&lock->waiters, iter.cur, lwWaitLink); diff --git a/src/backend/storage/lmgr/predicate.c b/src/backend/storage/lmgr/predicate.c index fbc0caca5c..508c8bb37f 100644 --- a/src/backend/storage/lmgr/predicate.c +++ b/src/backend/storage/lmgr/predicate.c @@ -3674,7 +3674,7 @@ ReleasePredicateLocks(bool isCommit, bool isReadOnlySafe) PGPROC *proc; proc = BackendIdGetProc(roXact->vxid.backendId); - ProcSendSignal(proc->pgprocno); + ProcSendSignal(GetPGProcNumber(proc)); } possibleUnsafeConflict = nextConflict; diff --git a/src/backend/storage/lmgr/proc.c b/src/backend/storage/lmgr/proc.c index 7f2111c6e0..1321786857 100644 --- a/src/backend/storage/lmgr/proc.c +++ b/src/backend/storage/lmgr/proc.c @@ -227,7 +227,6 @@ InitProcGlobal(void) InitSharedLatch(&(procs[i].procLatch)); LWLockInitialize(&(procs[i].fpInfoLock), LWTRANCHE_LOCK_FASTPATH); } - procs[i].pgprocno = i; /* * Newly created PGPROCs for normal backends, autovacuum and bgworkers @@ -1947,10 +1946,9 @@ BecomeLockGroupMember(PGPROC *leader, int pid) /* * Get lock protecting the group fields. Note LockHashPartitionLockByProc - * accesses leader->pgprocno in a PGPROC that might be free. This is safe - * because all PGPROCs' pgprocno fields are set during shared memory - * initialization and never change thereafter; so we will acquire the - * correct lock even if the leader PGPROC is in process of being recycled. + * takes a PGPROC that might be free, but it uses only its address. We + * will acquire the correct lock even if the leader PGPROC is in the + * process of being recycled. */ leader_lwlock = LockHashPartitionLockByProc(leader); LWLockAcquire(leader_lwlock, LW_EXCLUSIVE); diff --git a/src/backend/utils/adt/lockfuncs.c b/src/backend/utils/adt/lockfuncs.c index 5dc0a5882c..020f76e431 100644 --- a/src/backend/utils/adt/lockfuncs.c +++ b/src/backend/utils/adt/lockfuncs.c @@ -18,6 +18,7 @@ #include "funcapi.h" #include "miscadmin.h" #include "storage/predicate_internals.h" +#include "storage/proc.h" #include "utils/array.h" #include "utils/builtins.h" diff --git a/src/include/storage/lock.h b/src/include/storage/lock.h index 9b2a421c32..c8e1b0ce1a 100644 --- a/src/include/storage/lock.h +++ b/src/include/storage/lock.h @@ -531,7 +531,7 @@ typedef enum * used for a given lock group is determined by the group leader's pgprocno. */ #define LockHashPartitionLockByProc(leader_pgproc) \ - LockHashPartitionLock((leader_pgproc)->pgprocno) + LockHashPartitionLock(GetPGProcNumber(leader_pgproc)) /* * function prototypes diff --git a/src/include/storage/proc.h b/src/include/storage/proc.h index 7c1e2efe30..435876772b 100644 --- a/src/include/storage/proc.h +++ b/src/include/storage/proc.h @@ -147,7 +147,6 @@ struct PGPROC int pgxactoff; /* offset into various ProcGlobal->arrays with * data mirrored from this PGPROC */ - int pgprocno; /* These fields are zero while a backend is still starting up: */ BackendId backendId; /* This backend's backend ID (if assigned) */ @@ -311,6 +310,9 @@ extern PGDLLIMPORT PGPROC *MyProc; * When entering a PGPROC for 2PC transactions with ProcArrayAdd(), the data * in the dense arrays is initialized from the PGPROC while it already holds * ProcArrayLock. + * + * Shared memory data structures can refer to PGPROCs by index in allProcs. + * See GetPGProcNumber() and GetPGProcByNumber(). */ typedef struct PROC_HDR { @@ -362,6 +364,8 @@ extern PGPROC *PreparedXactProcs; /* Accessor for PGPROC given a pgprocno. */ #define GetPGProcByNumber(n) (&ProcGlobal->allProcs[(n)]) +/* Accessor for pgprocno given a pointer to PGPROC. */ +#define GetPGProcNumber(proc) ((proc) - ProcGlobal->allProcs) /* * We set aside some extra PGPROC structures for auxiliary processes, -- 2.30.2
