Re: Inconsistency in startup process's MyBackendId and procsignal array registration with ProcSignalInit() - Mailing list pgsql-hackers
From | Kyotaro Horiguchi |
---|---|
Subject | Re: Inconsistency in startup process's MyBackendId and procsignal array registration with ProcSignalInit() |
Date | |
Msg-id | 20211015.150057.1538271214250441184.horikyota.ntt@gmail.com Whole thread Raw |
In response to | Inconsistency in startup process's MyBackendId and procsignal array registration with ProcSignalInit() (Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>) |
Responses |
Re: Inconsistency in startup process's MyBackendId and procsignal array registration with ProcSignalInit()
Re: Inconsistency in startup process's MyBackendId and procsignal array registration with ProcSignalInit() |
List | pgsql-hackers |
At Thu, 14 Oct 2021 10:53:06 -0700, Andres Freund <andres@anarazel.de> wrote in > Hi, > > On 2021-10-14 17:28:34 +0900, Kyotaro Horiguchi wrote: > > At Wed, 13 Oct 2021 19:52:52 +0900 (JST), Kyotaro Horiguchi <horikyota.ntt@gmail.com> wrote in > > > Although needing a bit of care for the difference of invalid values > > > for both though, BackendId can be easily replaced with pgprocno almost > > > mechanically except sinvaladt. Therefore, we can confine the current > > > backend ID within sinvaladt isolating from other part. The ids > > > dedicated for sinvaladt can be packed to small range and perfomance > > > won't be damaged. > > FWIW, I don't actually think there's necessarily that strong a need for > density in sinvaladt. With a few relatively changes we can get rid of the O(n) > work in the most crucial paths. Right. So I left it for the "future:p > In https://www.postgresql.org/message-id/20210802171255.k4yv5cfqaqbuuy6f%40alap3.anarazel.de > I wrote: > > Another approach to deal with this could be to simply not do the O(n) work in > > SIInsertDataEntries(). It's not obvious that ->hasMessages is actually > > necessary - we could atomically read maxMsgNum without acquiring a lock > > instead of needing the per-backend ->hasMessages. I don't the density would > > be a relevant factor in SICleanupQueue(). > > This'd get rid of the need of density *and* make SIInsertDataEntries() > cheaper. Yes. So.. I tried that. The only part where memory-flush timing is crucial seems to be between writing messages and setting maxMsgNum. By placing memory barrier between them it seems *to me* we can read maxMsgNum safely without locks. I reread that thread and found we can get rid of O(N) behavior from two places, SIgnalVirtualTransaction and GetVirtualXIDsDelayingChkpt. Finally, I got rid of the siindex (the old BackendId) from sinvaladt.c at all. Still CleanupInvalidationState and SICleanupQueue has O(N) behavior but they are executed rarely or ends in a short time in the most cases. 0001: Reverses the proc freelist so that the backendid is assigned in the sane order. 0002: Replaces the current BackendId - that is generated by sinvaladt.c intending to pack the ids to a narrow range - with pgprocno in most of the tree. The old BackendID is now used only in sinvaladt.c 0003: Removes O(N) behavior from SIInsertDataEntries. I'm not sure it is correctly revised, though.. 0004: Gets rid of O(N), or reduce O(N^2) to O(N) of HaveVirtualXIDsDelayingChkpt and SignalVirtualTransaction. 0005: Gets rid of the old BackendID at all from sinvaladt.c. regards. -- Kyotaro Horiguchi NTT Open Source Software Center From 2b28c29d68eb2c37137aef66c7634b65aa640520 Mon Sep 17 00:00:00 2001 From: Kyotaro Horiguchi <horikyota.ntt@gmail.com> Date: Wed, 13 Oct 2021 10:31:26 +0900 Subject: [PATCH v2 1/5] procfreelist in ascending order --- src/backend/storage/lmgr/proc.c | 62 +++++++++++++++++++-------------- 1 file changed, 36 insertions(+), 26 deletions(-) diff --git a/src/backend/storage/lmgr/proc.c b/src/backend/storage/lmgr/proc.c index b7d9da0aa9..78e05976a4 100644 --- a/src/backend/storage/lmgr/proc.c +++ b/src/backend/storage/lmgr/proc.c @@ -163,6 +163,9 @@ InitProcGlobal(void) j; bool found; uint32 TotalProcs = MaxBackends + NUM_AUXILIARY_PROCS + max_prepared_xacts; + PGPROC **freelist; + int switchpoint; + int arraykind; /* Create the ProcGlobal shared structure */ ProcGlobal = (PROC_HDR *) @@ -214,6 +217,8 @@ InitProcGlobal(void) ProcGlobal->statusFlags = (uint8 *) ShmemAlloc(TotalProcs * sizeof(*ProcGlobal->statusFlags)); MemSet(ProcGlobal->statusFlags, 0, TotalProcs * sizeof(*ProcGlobal->statusFlags)); + switchpoint = 0; + arraykind = 0; for (i = 0; i < TotalProcs; i++) { /* Common initialization for all PGPROCs, regardless of type. */ @@ -239,33 +244,38 @@ InitProcGlobal(void) * linear search. PGPROCs for prepared transactions are added to a * free list by TwoPhaseShmemInit(). */ - if (i < MaxConnections) + if (i < MaxBackends) { - /* PGPROC for normal backend, add to freeProcs list */ - procs[i].links.next = (SHM_QUEUE *) ProcGlobal->freeProcs; - ProcGlobal->freeProcs = &procs[i]; - procs[i].procgloballist = &ProcGlobal->freeProcs; - } - else if (i < MaxConnections + autovacuum_max_workers + 1) - { - /* PGPROC for AV launcher/worker, add to autovacFreeProcs list */ - procs[i].links.next = (SHM_QUEUE *) ProcGlobal->autovacFreeProcs; - ProcGlobal->autovacFreeProcs = &procs[i]; - procs[i].procgloballist = &ProcGlobal->autovacFreeProcs; - } - else if (i < MaxConnections + autovacuum_max_workers + 1 + max_worker_processes) - { - /* PGPROC for bgworker, add to bgworkerFreeProcs list */ - procs[i].links.next = (SHM_QUEUE *) ProcGlobal->bgworkerFreeProcs; - ProcGlobal->bgworkerFreeProcs = &procs[i]; - procs[i].procgloballist = &ProcGlobal->bgworkerFreeProcs; - } - else if (i < MaxBackends) - { - /* PGPROC for walsender, add to walsenderFreeProcs list */ - procs[i].links.next = (SHM_QUEUE *) ProcGlobal->walsenderFreeProcs; - ProcGlobal->walsenderFreeProcs = &procs[i]; - procs[i].procgloballist = &ProcGlobal->walsenderFreeProcs; + if (i == switchpoint) + { + switch (arraykind++) + { + case 0: + freelist = &ProcGlobal->freeProcs; + switchpoint += MaxConnections; + break; + + case 1: + freelist = &ProcGlobal->autovacFreeProcs; + switchpoint += autovacuum_max_workers + 1; + break; + + case 2: + freelist = &ProcGlobal->bgworkerFreeProcs; + switchpoint += max_worker_processes; + break; + + case 3: + freelist = &ProcGlobal->walsenderFreeProcs; + } + + /* link the element to the just-switched freelist */ + *freelist = &procs[i]; + } + else + procs[i - 1].links.next = (SHM_QUEUE *) &procs[i]; + + procs[i].procgloballist = freelist; } /* Initialize myProcLocks[] shared memory queues. */ -- 2.27.0 From d7cc2b6b95dd90683b47e5692b2acbc370223ada Mon Sep 17 00:00:00 2001 From: Kyotaro Horiguchi <horikyota.ntt@gmail.com> Date: Thu, 14 Oct 2021 17:05:24 +0900 Subject: [PATCH v2 2/5] Remove BackendId BackendId was generated by sinvaladt.c and widely used as ids packed in narrow range. However, the characteristics of narrow-packed is not required other than sinvaladt.c and the use of the id in other places rather harm. Get rid of the backend id in most of the tree and use pgprocno as new BackendId. However, sinvaladt still needs such packed id so the old backend id is confined to the module. --- src/backend/access/transam/multixact.c | 23 +++++----- src/backend/access/transam/twophase.c | 4 +- src/backend/access/transam/xact.c | 2 +- src/backend/catalog/namespace.c | 2 +- src/backend/commands/async.c | 21 +++++---- src/backend/commands/indexcmds.c | 3 +- src/backend/postmaster/auxprocess.c | 2 +- src/backend/postmaster/pgstat.c | 2 +- src/backend/storage/ipc/procarray.c | 9 ++-- src/backend/storage/ipc/procsignal.c | 25 ++++------- src/backend/storage/ipc/sinvaladt.c | 47 ++++----------------- src/backend/storage/ipc/standby.c | 2 +- src/backend/storage/lmgr/lmgr.c | 3 +- src/backend/storage/lmgr/lock.c | 20 ++++----- src/backend/storage/lmgr/proc.c | 29 ++++++++++--- src/backend/utils/activity/backend_status.c | 22 +--------- src/backend/utils/adt/dbsize.c | 5 ++- src/backend/utils/adt/mcxtfuncs.c | 2 +- src/backend/utils/cache/relcache.c | 1 + src/backend/utils/error/elog.c | 10 ++--- src/backend/utils/init/globals.c | 2 - src/backend/utils/init/postinit.c | 7 +-- src/backend/utils/time/snapmgr.c | 4 +- src/include/storage/backendid.h | 3 +- src/include/storage/lock.h | 6 +-- src/include/storage/proc.h | 11 +++-- src/include/storage/procsignal.h | 4 +- src/include/storage/smgr.h | 2 +- src/include/utils/rel.h | 2 +- 29 files changed, 116 insertions(+), 159 deletions(-) diff --git a/src/backend/access/transam/multixact.c b/src/backend/access/transam/multixact.c index e6c70ed0bc..585dc50858 100644 --- a/src/backend/access/transam/multixact.c +++ b/src/backend/access/transam/multixact.c @@ -238,9 +238,9 @@ typedef struct MultiXactStateData * immediately following the MultiXactStateData struct. Each is indexed by * BackendId. * - * In both arrays, there's a slot for all normal backends (1..MaxBackends) - * followed by a slot for max_prepared_xacts prepared transactions. Valid - * BackendIds start from 1; element zero of each array is never used. + * In both arrays, there's a slot for all normal backends + * (0..MaxBackends-1) followed by a slot for max_prepared_xacts prepared + * transactions. * * OldestMemberMXactId[k] is the oldest MultiXactId each backend's current * transaction(s) could possibly be a member of, or InvalidMultiXactId @@ -283,9 +283,9 @@ typedef struct MultiXactStateData /* * Last element of OldestMemberMXactId and OldestVisibleMXactId arrays. - * Valid elements are (1..MaxOldestSlot); element 0 is never used. + * Valid elements are (0..MaxOldestSlot). */ -#define MaxOldestSlot (MaxBackends + max_prepared_xacts) +#define MaxOldestSlot (MaxBackends + max_prepared_xacts - 1) /* Pointers to the state data in shared memory */ static MultiXactStateData *MultiXactState; @@ -697,7 +697,7 @@ MultiXactIdSetOldestVisible(void) if (oldestMXact < FirstMultiXactId) oldestMXact = FirstMultiXactId; - for (i = 1; i <= MaxOldestSlot; i++) + for (i = 0 ; i <= MaxOldestSlot; i++) { MultiXactId thisoldest = OldestMemberMXactId[i]; @@ -1828,10 +1828,10 @@ MultiXactShmemSize(void) { Size size; - /* We need 2*MaxOldestSlot + 1 perBackendXactIds[] entries */ + /* We need 2*(MaxOldestSlot + 1) + 1 perBackendXactIds[] entries */ #define SHARED_MULTIXACT_STATE_SIZE \ add_size(offsetof(MultiXactStateData, perBackendXactIds) + sizeof(MultiXactId), \ - mul_size(sizeof(MultiXactId) * 2, MaxOldestSlot)) + mul_size(sizeof(MultiXactId) * 2, MaxOldestSlot + 1)) size = SHARED_MULTIXACT_STATE_SIZE; size = add_size(size, SimpleLruShmemSize(NUM_MULTIXACTOFFSET_BUFFERS, 0)); @@ -1878,11 +1878,10 @@ MultiXactShmemInit(void) Assert(found); /* - * Set up array pointers. Note that perBackendXactIds[0] is wasted space - * since we only use indexes 1..MaxOldestSlot in each array. + * Set up array pointers. */ OldestMemberMXactId = MultiXactState->perBackendXactIds; - OldestVisibleMXactId = OldestMemberMXactId + MaxOldestSlot; + OldestVisibleMXactId = OldestMemberMXactId + MaxOldestSlot + 1; } /* @@ -2525,7 +2524,7 @@ GetOldestMultiXactId(void) nextMXact = FirstMultiXactId; oldestMXact = nextMXact; - for (i = 1; i <= MaxOldestSlot; i++) + for (i = 0; i <= MaxOldestSlot; i++) { MultiXactId thisoldest; diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c index 2156de187c..76d5bb55d6 100644 --- a/src/backend/access/transam/twophase.c +++ b/src/backend/access/transam/twophase.c @@ -293,7 +293,7 @@ TwoPhaseShmemInit(void) * prepared transaction. Currently multixact.c uses that * technique. */ - gxacts[i].dummyBackendId = MaxBackends + 1 + i; + gxacts[i].dummyBackendId = MaxBackends + i; } } else @@ -459,14 +459,12 @@ MarkAsPreparingGuts(GlobalTransaction gxact, TransactionId xid, const char *gid, proc->pgprocno = gxact->pgprocno; SHMQueueElemInit(&(proc->links)); proc->waitStatus = PROC_WAIT_STATUS_OK; - /* We set up the gxact's VXID as InvalidBackendId/XID */ proc->lxid = (LocalTransactionId) xid; proc->xid = xid; Assert(proc->xmin == InvalidTransactionId); proc->delayChkpt = false; proc->statusFlags = 0; proc->pid = 0; - proc->backendId = InvalidBackendId; proc->databaseId = databaseid; proc->roleId = owner; proc->tempNamespaceId = InvalidOid; diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c index 4cc38f0d85..9a8b0686bd 100644 --- a/src/backend/access/transam/xact.c +++ b/src/backend/access/transam/xact.c @@ -2020,7 +2020,7 @@ StartTransaction(void) * Advertise it in the proc array. We assume assignment of * localTransactionId is atomic, and the backendId should be set already. */ - Assert(MyProc->backendId == vxid.backendId); + Assert(MyProc->pgprocno == vxid.backendId); MyProc->lxid = vxid.localTransactionId; TRACE_POSTGRESQL_TRANSACTION_START(vxid.localTransactionId); diff --git a/src/backend/catalog/namespace.c b/src/backend/catalog/namespace.c index 4de8400fd0..f450c46f4d 100644 --- a/src/backend/catalog/namespace.c +++ b/src/backend/catalog/namespace.c @@ -3293,7 +3293,7 @@ checkTempNamespaceStatus(Oid namespaceId) return TEMP_NAMESPACE_NOT_TEMP; /* Is the backend alive? */ - proc = BackendIdGetProc(backendId); + proc = GetProcIfAlive(backendId); if (proc == NULL) return TEMP_NAMESPACE_IDLE; diff --git a/src/backend/commands/async.c b/src/backend/commands/async.c index 8557008545..e007c17f51 100644 --- a/src/backend/commands/async.c +++ b/src/backend/commands/async.c @@ -272,8 +272,8 @@ typedef struct QueueBackendStatus * NotifyQueueTailLock, then NotifyQueueLock, and lastly NotifySLRULock. * * Each backend uses the backend[] array entry with index equal to its - * BackendId (which can range from 1 to MaxBackends). We rely on this to make - * SendProcSignal fast. + * BackendId (which can range from 0 to MaxBackends - 1). We rely on this to + * make SendProcSignal fast. * * The backend[] array entries for actively-listening backends are threaded * together using firstListener and the nextListener links, so that we can @@ -1122,7 +1122,8 @@ Exec_ListenPreCommit(void) head = QUEUE_HEAD; max = QUEUE_TAIL; prevListener = InvalidBackendId; - for (BackendId i = QUEUE_FIRST_LISTENER; i > 0; i = QUEUE_NEXT_LISTENER(i)) + for (BackendId i = QUEUE_FIRST_LISTENER; i != InvalidBackendId ; + i = QUEUE_NEXT_LISTENER(i)) { if (QUEUE_BACKEND_DBOID(i) == MyDatabaseId) max = QUEUE_POS_MAX(max, QUEUE_BACKEND_POS(i)); @@ -1134,7 +1135,7 @@ Exec_ListenPreCommit(void) QUEUE_BACKEND_PID(MyBackendId) = MyProcPid; QUEUE_BACKEND_DBOID(MyBackendId) = MyDatabaseId; /* Insert backend into list of listeners at correct position */ - if (prevListener > 0) + if (prevListener != InvalidBackendId) { QUEUE_NEXT_LISTENER(MyBackendId) = QUEUE_NEXT_LISTENER(prevListener); QUEUE_NEXT_LISTENER(prevListener) = MyBackendId; @@ -1281,7 +1282,8 @@ asyncQueueUnregister(void) QUEUE_FIRST_LISTENER = QUEUE_NEXT_LISTENER(MyBackendId); else { - for (BackendId i = QUEUE_FIRST_LISTENER; i > 0; i = QUEUE_NEXT_LISTENER(i)) + for (BackendId i = QUEUE_FIRST_LISTENER; i != InvalidBackendId ; + i = QUEUE_NEXT_LISTENER(i)) { if (QUEUE_NEXT_LISTENER(i) == MyBackendId) { @@ -1590,7 +1592,8 @@ asyncQueueFillWarning(void) QueuePosition min = QUEUE_HEAD; int32 minPid = InvalidPid; - for (BackendId i = QUEUE_FIRST_LISTENER; i > 0; i = QUEUE_NEXT_LISTENER(i)) + for (BackendId i = QUEUE_FIRST_LISTENER; i != InvalidBackendId; + i = QUEUE_NEXT_LISTENER(i)) { Assert(QUEUE_BACKEND_PID(i) != InvalidPid); min = QUEUE_POS_MIN(min, QUEUE_BACKEND_POS(i)); @@ -1646,7 +1649,8 @@ SignalBackends(void) count = 0; LWLockAcquire(NotifyQueueLock, LW_EXCLUSIVE); - for (BackendId i = QUEUE_FIRST_LISTENER; i > 0; i = QUEUE_NEXT_LISTENER(i)) + for (BackendId i = QUEUE_FIRST_LISTENER; i != InvalidBackendId ; + i = QUEUE_NEXT_LISTENER(i)) { int32 pid = QUEUE_BACKEND_PID(i); QueuePosition pos; @@ -2183,7 +2187,8 @@ asyncQueueAdvanceTail(void) */ LWLockAcquire(NotifyQueueLock, LW_EXCLUSIVE); min = QUEUE_HEAD; - for (BackendId i = QUEUE_FIRST_LISTENER; i > 0; i = QUEUE_NEXT_LISTENER(i)) + for (int i = QUEUE_FIRST_LISTENER; i != InvalidBackendId ; + i = QUEUE_NEXT_LISTENER(i)) { Assert(QUEUE_BACKEND_PID(i) != InvalidPid); min = QUEUE_POS_MIN(min, QUEUE_BACKEND_POS(i)); diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c index c14ca27c5e..fa70eed559 100644 --- a/src/backend/commands/indexcmds.c +++ b/src/backend/commands/indexcmds.c @@ -463,7 +463,8 @@ WaitForOlderSnapshots(TransactionId limitXmin, bool progress) /* If requested, publish who we're going to wait for. */ if (progress) { - PGPROC *holder = BackendIdGetProc(old_snapshots[i].backendId); + PGPROC *holder = + &ProcGlobal->allProcs[old_snapshots[i].backendId]; if (holder) pgstat_progress_update_param(PROGRESS_WAITFOR_CURRENT_PID, diff --git a/src/backend/postmaster/auxprocess.c b/src/backend/postmaster/auxprocess.c index 7452f908b2..4c7784b036 100644 --- a/src/backend/postmaster/auxprocess.c +++ b/src/backend/postmaster/auxprocess.c @@ -116,7 +116,7 @@ AuxiliaryProcessMain(AuxProcType auxtype) * This will need rethinking if we ever want more than one of a particular * auxiliary process type. */ - ProcSignalInit(MaxBackends + MyAuxProcType + 1); + ProcSignalInit(); /* * Auxiliary processes don't run transactions, but they may need a diff --git a/src/backend/postmaster/pgstat.c b/src/backend/postmaster/pgstat.c index b7d0fbaefd..b1cd23f661 100644 --- a/src/backend/postmaster/pgstat.c +++ b/src/backend/postmaster/pgstat.c @@ -54,7 +54,7 @@ #include "postmaster/postmaster.h" #include "replication/slot.h" #include "replication/walsender.h" -#include "storage/backendid.h" +//#include "storage/backendid.h" #include "storage/dsm.h" #include "storage/fd.h" #include "storage/ipc.h" diff --git a/src/backend/storage/ipc/procarray.c b/src/backend/storage/ipc/procarray.c index bd3c7a47fe..100c0dae8c 100644 --- a/src/backend/storage/ipc/procarray.c +++ b/src/backend/storage/ipc/procarray.c @@ -2592,7 +2592,7 @@ ProcArrayInstallImportedXmin(TransactionId xmin, continue; /* We are only interested in the specific virtual transaction. */ - if (proc->backendId != sourcevxid->backendId) + if (proc->pgprocno != sourcevxid->backendId) continue; if (proc->lxid != sourcevxid->localTransactionId) continue; @@ -3454,7 +3454,7 @@ SignalVirtualTransaction(VirtualTransactionId vxid, ProcSignalReason sigmode, * Kill the pid if it's still here. If not, that's what we * wanted so ignore any errors. */ - (void) SendProcSignal(pid, sigmode, vxid.backendId); + (void) SendProcSignal(pid, sigmode, proc->pgprocno); } break; } @@ -3604,11 +3604,8 @@ CancelDBBackends(Oid databaseid, ProcSignalReason sigmode, bool conflictPending) if (databaseid == InvalidOid || proc->databaseId == databaseid) { - VirtualTransactionId procvxid; pid_t pid; - GET_VXID_FROM_PGPROC(procvxid, *proc); - proc->recoveryConflictPending = conflictPending; pid = proc->pid; if (pid != 0) @@ -3617,7 +3614,7 @@ CancelDBBackends(Oid databaseid, ProcSignalReason sigmode, bool conflictPending) * Kill the pid if it's still here. If not, that's what we * wanted so ignore any errors. */ - (void) SendProcSignal(pid, sigmode, procvxid.backendId); + (void) SendProcSignal(pid, sigmode, proc->pgprocno); } } } diff --git a/src/backend/storage/ipc/procsignal.c b/src/backend/storage/ipc/procsignal.c index defb75aa26..ffe6939780 100644 --- a/src/backend/storage/ipc/procsignal.c +++ b/src/backend/storage/ipc/procsignal.c @@ -81,9 +81,8 @@ typedef struct } ProcSignalHeader; /* - * We reserve a slot for each possible BackendId, plus one for each - * possible auxiliary process type. (This scheme assumes there is not - * more than one of any auxiliary process type at a time.) + * We reserve a slot for PGPROCs for backends and auxiliary processes. Not all + * of auxiliary processes use this but allocate for them for safety. */ #define NumProcSignalSlots (MaxBackends + NUM_AUXPROCTYPES) @@ -153,24 +152,19 @@ ProcSignalShmemInit(void) /* * ProcSignalInit * Register the current process in the procsignal array - * - * The passed index should be my BackendId if the process has one, - * or MaxBackends + aux process type if not. */ void -ProcSignalInit(int pss_idx) +ProcSignalInit(void) { ProcSignalSlot *slot; uint64 barrier_generation; - Assert(pss_idx >= 1 && pss_idx <= NumProcSignalSlots); - - slot = &ProcSignal->psh_slot[pss_idx - 1]; + slot = &ProcSignal->psh_slot[MyBackendId]; /* sanity check */ if (slot->pss_pid != 0) elog(LOG, "process %d taking over ProcSignal slot %d, but it's not empty", - MyProcPid, pss_idx); + MyProcPid, MyBackendId); /* Clear out any leftover signal reasons */ MemSet(slot->pss_signalFlags, 0, NUM_PROCSIGNALS * sizeof(sig_atomic_t)); @@ -199,7 +193,7 @@ ProcSignalInit(int pss_idx) MyProcSignalSlot = slot; /* Set up to release the slot on process exit */ - on_shmem_exit(CleanupProcSignalState, Int32GetDatum(pss_idx)); + on_shmem_exit(CleanupProcSignalState, (Datum) 0); } /* @@ -211,10 +205,9 @@ ProcSignalInit(int pss_idx) static void CleanupProcSignalState(int status, Datum arg) { - int pss_idx = DatumGetInt32(arg); ProcSignalSlot *slot; - slot = &ProcSignal->psh_slot[pss_idx - 1]; + slot = &ProcSignal->psh_slot[MyBackendId]; Assert(slot == MyProcSignalSlot); /* @@ -232,7 +225,7 @@ CleanupProcSignalState(int status, Datum arg) * infinite loop trying to exit */ elog(LOG, "process %d releasing ProcSignal slot %d, but it contains %d", - MyProcPid, pss_idx, (int) slot->pss_pid); + MyProcPid, MyProcPid, (int) slot->pss_pid); return; /* XXX better to zero the slot anyway? */ } @@ -264,7 +257,7 @@ SendProcSignal(pid_t pid, ProcSignalReason reason, BackendId backendId) if (backendId != InvalidBackendId) { - slot = &ProcSignal->psh_slot[backendId - 1]; + slot = &ProcSignal->psh_slot[backendId]; /* * Note: Since there's no locking, it's possible that the target diff --git a/src/backend/storage/ipc/sinvaladt.c b/src/backend/storage/ipc/sinvaladt.c index 946bd8e3cb..a90e9920e7 100644 --- a/src/backend/storage/ipc/sinvaladt.c +++ b/src/backend/storage/ipc/sinvaladt.c @@ -19,7 +19,6 @@ #include "access/transam.h" #include "miscadmin.h" -#include "storage/backendid.h" #include "storage/ipc.h" #include "storage/proc.h" #include "storage/procsignal.h" @@ -157,7 +156,7 @@ typedef struct ProcState /* * Next LocalTransactionId to use for each idle backend slot. We keep * this here because it is indexed by BackendId and it is convenient to - * copy the value to and from local memory when MyBackendId is set. It's + * copy the value to and from local memory when MybackendId is set. It's * meaningless in an active ProcState entry. */ LocalTransactionId nextLXID; @@ -195,6 +194,7 @@ static LocalTransactionId nextLocalTransactionId; static void CleanupInvalidationState(int status, Datum arg); +static int siindex; /* * SInvalShmemSize --- return shared-memory space needed @@ -290,7 +290,7 @@ SharedInvalBackendInit(bool sendOnly) /* * out of procState slots: MaxBackends exceeded -- report normally */ - MyBackendId = InvalidBackendId; + siindex = -1; LWLockRelease(SInvalWriteLock); ereport(FATAL, (errcode(ERRCODE_TOO_MANY_CONNECTIONS), @@ -298,10 +298,7 @@ SharedInvalBackendInit(bool sendOnly) } } - MyBackendId = (stateP - &segP->procState[0]) + 1; - - /* Advertise assigned backend ID in MyProc */ - MyProc->backendId = MyBackendId; + siindex = (stateP - &segP->procState[0]); /* Fetch next local transaction ID into local memory */ nextLocalTransactionId = stateP->nextLXID; @@ -320,7 +317,7 @@ SharedInvalBackendInit(bool sendOnly) /* register exit routine to mark my entry inactive at exit */ on_shmem_exit(CleanupInvalidationState, PointerGetDatum(segP)); - elog(DEBUG4, "my backend ID is %d", MyBackendId); + elog(DEBUG4, "my SI slot index is %d", siindex); } /* @@ -342,7 +339,7 @@ CleanupInvalidationState(int status, Datum arg) LWLockAcquire(SInvalWriteLock, LW_EXCLUSIVE); - stateP = &segP->procState[MyBackendId - 1]; + stateP = &segP->procState[siindex]; /* Update next local transaction ID for next holder of this backendID */ stateP->nextLXID = nextLocalTransactionId; @@ -365,34 +362,6 @@ CleanupInvalidationState(int status, Datum arg) LWLockRelease(SInvalWriteLock); } -/* - * BackendIdGetProc - * Get the PGPROC structure for a backend, given the backend ID. - * The result may be out of date arbitrarily quickly, so the caller - * must be careful about how this information is used. NULL is - * returned if the backend is not active. - */ -PGPROC * -BackendIdGetProc(int backendID) -{ - PGPROC *result = NULL; - SISeg *segP = shmInvalBuffer; - - /* Need to lock out additions/removals of backends */ - LWLockAcquire(SInvalWriteLock, LW_SHARED); - - if (backendID > 0 && backendID <= segP->lastBackend) - { - ProcState *stateP = &segP->procState[backendID - 1]; - - result = stateP->proc; - } - - LWLockRelease(SInvalWriteLock); - - return result; -} - /* * BackendIdGetTransactionIds * Get the xid and xmin of the backend. The result may be out of date @@ -541,7 +510,7 @@ SIGetDataEntries(SharedInvalidationMessage *data, int datasize) int n; segP = shmInvalBuffer; - stateP = &segP->procState[MyBackendId - 1]; + stateP = &segP->procState[siindex]; /* * Before starting to take locks, do a quick, unlocked test to see whether @@ -730,7 +699,7 @@ SICleanupQueue(bool callerHasWriteLock, int minFree) if (needSig) { pid_t his_pid = needSig->procPid; - BackendId his_backendId = (needSig - &segP->procState[0]) + 1; + int his_backendId = needSig->proc->pgprocno; needSig->signaled = true; LWLockRelease(SInvalReadLock); diff --git a/src/backend/storage/ipc/standby.c b/src/backend/storage/ipc/standby.c index b17326bc20..032f0428aa 100644 --- a/src/backend/storage/ipc/standby.c +++ b/src/backend/storage/ipc/standby.c @@ -274,7 +274,7 @@ LogRecoveryConflict(ProcSignalReason reason, TimestampTz wait_start, vxids = wait_list; while (VirtualTransactionIdIsValid(*vxids)) { - PGPROC *proc = BackendIdGetProc(vxids->backendId); + PGPROC *proc = &ProcGlobal->allProcs[vxids->backendId]; /* proc can be NULL if the target backend is not active */ if (proc) diff --git a/src/backend/storage/lmgr/lmgr.c b/src/backend/storage/lmgr/lmgr.c index cdf2266d6d..f0208531e0 100644 --- a/src/backend/storage/lmgr/lmgr.c +++ b/src/backend/storage/lmgr/lmgr.c @@ -918,7 +918,8 @@ WaitForLockersMultiple(List *locktags, LOCKMODE lockmode, bool progress) /* If requested, publish who we're going to wait for. */ if (progress) { - PGPROC *holder = BackendIdGetProc(lockholders->backendId); + PGPROC *holder = + &ProcGlobal->allProcs[lockholders->backendId]; if (holder) pgstat_progress_update_param(PROGRESS_WAITFOR_CURRENT_PID, diff --git a/src/backend/storage/lmgr/lock.c b/src/backend/storage/lmgr/lock.c index 364654e106..3edb7d6fd5 100644 --- a/src/backend/storage/lmgr/lock.c +++ b/src/backend/storage/lmgr/lock.c @@ -3695,7 +3695,7 @@ GetLockStatusData(void) proc->fpRelId[f]); instance->holdMask = lockbits << FAST_PATH_LOCKNUMBER_OFFSET; instance->waitLockMode = NoLock; - instance->backend = proc->backendId; + instance->backend = proc->pgprocno; instance->lxid = proc->lxid; instance->pid = proc->pid; instance->leaderPid = proc->pid; @@ -3722,14 +3722,14 @@ GetLockStatusData(void) repalloc(data->locks, sizeof(LockInstanceData) * els); } - vxid.backendId = proc->backendId; + vxid.backendId = proc->pgprocno; vxid.localTransactionId = proc->fpLocalTransactionId; instance = &data->locks[el]; SET_LOCKTAG_VIRTUALTRANSACTION(instance->locktag, vxid); instance->holdMask = LOCKBIT_ON(ExclusiveLock); instance->waitLockMode = NoLock; - instance->backend = proc->backendId; + instance->backend = proc->pgprocno; instance->lxid = proc->lxid; instance->pid = proc->pid; instance->leaderPid = proc->pid; @@ -3782,7 +3782,7 @@ GetLockStatusData(void) instance->waitLockMode = proc->waitLockMode; else instance->waitLockMode = NoLock; - instance->backend = proc->backendId; + instance->backend = proc->pgprocno; instance->lxid = proc->lxid; instance->pid = proc->pid; instance->leaderPid = proclock->groupLeader->pid; @@ -3961,7 +3961,7 @@ GetSingleProcBlockerStatusData(PGPROC *blocked_proc, BlockedProcsData *data) instance->waitLockMode = proc->waitLockMode; else instance->waitLockMode = NoLock; - instance->backend = proc->backendId; + instance->backend = proc->pgprocno; instance->lxid = proc->lxid; instance->pid = proc->pid; instance->leaderPid = proclock->groupLeader->pid; @@ -4475,7 +4475,7 @@ VirtualXactLockTableInsert(VirtualTransactionId vxid) LWLockAcquire(&MyProc->fpInfoLock, LW_EXCLUSIVE); - Assert(MyProc->backendId == vxid.backendId); + Assert(MyProc->pgprocno == vxid.backendId); Assert(MyProc->fpLocalTransactionId == InvalidLocalTransactionId); Assert(MyProc->fpVXIDLock == false); @@ -4497,8 +4497,6 @@ VirtualXactLockTableCleanup(void) bool fastpath; LocalTransactionId lxid; - Assert(MyProc->backendId != InvalidBackendId); - /* * Clean up shared memory state. */ @@ -4520,7 +4518,7 @@ VirtualXactLockTableCleanup(void) VirtualTransactionId vxid; LOCKTAG locktag; - vxid.backendId = MyBackendId; + vxid.backendId = MyProc->pgprocno; vxid.localTransactionId = lxid; SET_LOCKTAG_VIRTUALTRANSACTION(locktag, vxid); @@ -4571,7 +4569,7 @@ VirtualXactLock(VirtualTransactionId vxid, bool wait) * relevant lxid is no longer running here, that's enough to prove that * it's no longer running anywhere. */ - proc = BackendIdGetProc(vxid.backendId); + proc = &ProcGlobal->allProcs[vxid.backendId]; if (proc == NULL) return true; @@ -4583,7 +4581,7 @@ VirtualXactLock(VirtualTransactionId vxid, bool wait) LWLockAcquire(&proc->fpInfoLock, LW_EXCLUSIVE); /* If the transaction has ended, our work here is done. */ - if (proc->backendId != vxid.backendId + if (proc->pgprocno != vxid.backendId || proc->fpLocalTransactionId != vxid.localTransactionId) { LWLockRelease(&proc->fpInfoLock); diff --git a/src/backend/storage/lmgr/proc.c b/src/backend/storage/lmgr/proc.c index 78e05976a4..0ee6627e20 100644 --- a/src/backend/storage/lmgr/proc.c +++ b/src/backend/storage/lmgr/proc.c @@ -386,6 +386,8 @@ InitProcess(void) if (IsUnderPostmaster && !IsAutoVacuumLauncherProcess()) MarkPostmasterChildActive(); + MyBackendId = MyProc->pgprocno; + /* * Initialize all fields of MyProc, except for those previously * initialized by InitProcGlobal. @@ -398,14 +400,13 @@ InitProcess(void) MyProc->xid = InvalidTransactionId; MyProc->xmin = InvalidTransactionId; MyProc->pid = MyProcPid; - /* backendId, databaseId and roleId will be filled in later */ - MyProc->backendId = InvalidBackendId; + /* databaseId and roleId will be filled in later */ MyProc->databaseId = InvalidOid; MyProc->roleId = InvalidOid; MyProc->tempNamespaceId = InvalidOid; MyProc->isBackgroundWorker = IsBackgroundWorker; MyProc->delayChkpt = false; - MyProc->statusFlags = 0; + MyProc->statusFlags = PROC_IS_ACTIVE; /* NB -- autovac launcher intentionally does not set IS_AUTOVACUUM */ if (IsAutoVacuumWorkerProcess()) MyProc->statusFlags |= PROC_IS_AUTOVACUUM; @@ -570,6 +571,7 @@ InitAuxiliaryProcess(void) ((volatile PGPROC *) auxproc)->pid = MyProcPid; MyProc = auxproc; + MyBackendId = MyProc->pgprocno; SpinLockRelease(ProcStructLock); @@ -584,13 +586,12 @@ InitAuxiliaryProcess(void) MyProc->fpLocalTransactionId = InvalidLocalTransactionId; MyProc->xid = InvalidTransactionId; MyProc->xmin = InvalidTransactionId; - MyProc->backendId = InvalidBackendId; MyProc->databaseId = InvalidOid; MyProc->roleId = InvalidOid; MyProc->tempNamespaceId = InvalidOid; MyProc->isBackgroundWorker = IsBackgroundWorker; MyProc->delayChkpt = false; - MyProc->statusFlags = 0; + MyProc->statusFlags = PROC_IS_ACTIVE; MyProc->lwWaiting = false; MyProc->lwWaitMode = 0; MyProc->waitLock = NULL; @@ -913,6 +914,9 @@ ProcKill(int code, Datum arg) MyProc = NULL; DisownLatch(&proc->procLatch); + /* mark this process dead */ + proc->statusFlags &= ~PROC_IS_ACTIVE; + procgloballist = proc->procgloballist; SpinLockAcquire(ProcStructLock); @@ -985,6 +989,7 @@ AuxiliaryProcKill(int code, Datum arg) /* Mark auxiliary proc no longer in use */ proc->pid = 0; + proc->statusFlags &= ~PROC_IS_ACTIVE; /* Update shared estimate of spins_per_delay */ ProcGlobal->spins_per_delay = update_spins_per_delay(ProcGlobal->spins_per_delay); @@ -2020,3 +2025,17 @@ BecomeLockGroupMember(PGPROC *leader, int pid) return ok; } + +/* + * Return PGPROC if it is alive. + */ +PGPROC * +GetProcIfAlive(BackendId backend) +{ + PGPROC *proc = &ProcGlobal->allProcs[backend]; + + if (proc->statusFlags & PROC_IS_ACTIVE) + return proc; + + return NULL; +} diff --git a/src/backend/utils/activity/backend_status.c b/src/backend/utils/activity/backend_status.c index 7229598822..0490a3a8b2 100644 --- a/src/backend/utils/activity/backend_status.c +++ b/src/backend/utils/activity/backend_status.c @@ -249,26 +249,8 @@ void pgstat_beinit(void) { /* Initialize MyBEEntry */ - if (MyBackendId != InvalidBackendId) - { - Assert(MyBackendId >= 1 && MyBackendId <= MaxBackends); - MyBEEntry = &BackendStatusArray[MyBackendId - 1]; - } - else - { - /* Must be an auxiliary process */ - Assert(MyAuxProcType != NotAnAuxProcess); - - /* - * Assign the MyBEEntry for an auxiliary process. Since it doesn't - * have a BackendId, the slot is statically allocated based on the - * auxiliary process type (MyAuxProcType). Backends use slots indexed - * in the range from 1 to MaxBackends (inclusive), so we use - * MaxBackends + AuxBackendType + 1 as the index of the slot for an - * auxiliary process. - */ - MyBEEntry = &BackendStatusArray[MaxBackends + MyAuxProcType]; - } + Assert(MyBackendId >= 0 && MyBackendId < NumBackendStatSlots); + MyBEEntry = &BackendStatusArray[MyBackendId]; /* Set up a process-exit hook to clean up */ on_shmem_exit(pgstat_beshutdown_hook, 0); diff --git a/src/backend/utils/adt/dbsize.c b/src/backend/utils/adt/dbsize.c index d5a7fb13f3..16acb0c335 100644 --- a/src/backend/utils/adt/dbsize.c +++ b/src/backend/utils/adt/dbsize.c @@ -22,6 +22,7 @@ #include "commands/dbcommands.h" #include "commands/tablespace.h" #include "miscadmin.h" +#include "storage/backendid.h" #include "storage/fd.h" #include "utils/acl.h" #include "utils/builtins.h" @@ -292,7 +293,7 @@ pg_tablespace_size_name(PG_FUNCTION_ARGS) * is no check here or at the call sites for that. */ static int64 -calculate_relation_size(RelFileNode *rfn, BackendId backend, ForkNumber forknum) +calculate_relation_size(RelFileNode *rfn, int backend, ForkNumber forknum) { int64 totalsize = 0; char *relationpath; @@ -925,7 +926,7 @@ pg_relation_filepath(PG_FUNCTION_ARGS) HeapTuple tuple; Form_pg_class relform; RelFileNode rnode; - BackendId backend; + int backend; char *path; tuple = SearchSysCache1(RELOID, ObjectIdGetDatum(relid)); diff --git a/src/backend/utils/adt/mcxtfuncs.c b/src/backend/utils/adt/mcxtfuncs.c index 0d52613bc3..fca87448cf 100644 --- a/src/backend/utils/adt/mcxtfuncs.c +++ b/src/backend/utils/adt/mcxtfuncs.c @@ -205,7 +205,7 @@ pg_log_backend_memory_contexts(PG_FUNCTION_ARGS) PG_RETURN_BOOL(false); } - if (SendProcSignal(pid, PROCSIG_LOG_MEMORY_CONTEXT, proc->backendId) < 0) + if (SendProcSignal(pid, PROCSIG_LOG_MEMORY_CONTEXT, proc->pgprocno) < 0) { /* Again, just a warning to allow loops */ ereport(WARNING, diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c index 13d9994af3..00036ec9c0 100644 --- a/src/backend/utils/cache/relcache.c +++ b/src/backend/utils/cache/relcache.c @@ -73,6 +73,7 @@ #include "optimizer/optimizer.h" #include "rewrite/rewriteDefine.h" #include "rewrite/rowsecurity.h" +#include "storage/backendid.h" #include "storage/lmgr.h" #include "storage/smgr.h" #include "utils/array.h" diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c index f33729513a..0f927751d7 100644 --- a/src/backend/utils/error/elog.c +++ b/src/backend/utils/error/elog.c @@ -2685,18 +2685,18 @@ log_line_prefix(StringInfo buf, ErrorData *edata) break; case 'v': /* keep VXID format in sync with lockfuncs.c */ - if (MyProc != NULL && MyProc->backendId != InvalidBackendId) + if (MyProc != NULL && MyProc->pgprocno != InvalidBackendId) { if (padding != 0) { char strfbuf[128]; snprintf(strfbuf, sizeof(strfbuf) - 1, "%d/%u", - MyProc->backendId, MyProc->lxid); + MyProc->pgprocno, MyProc->lxid); appendStringInfo(buf, "%*s", padding, strfbuf); } else - appendStringInfo(buf, "%d/%u", MyProc->backendId, MyProc->lxid); + appendStringInfo(buf, "%d/%u", MyProc->pgprocno, MyProc->lxid); } else if (padding != 0) appendStringInfoSpaces(buf, @@ -2860,8 +2860,8 @@ write_csvlog(ErrorData *edata) /* Virtual transaction id */ /* keep VXID format in sync with lockfuncs.c */ - if (MyProc != NULL && MyProc->backendId != InvalidBackendId) - appendStringInfo(&buf, "%d/%u", MyProc->backendId, MyProc->lxid); + if (MyProc != NULL && MyProc->pgprocno != InvalidBackendId) + appendStringInfo(&buf, "%d/%u", MyProc->pgprocno, MyProc->lxid); appendStringInfoChar(&buf, ','); /* Transaction id */ diff --git a/src/backend/utils/init/globals.c b/src/backend/utils/init/globals.c index 381d9e548d..4ba1914472 100644 --- a/src/backend/utils/init/globals.c +++ b/src/backend/utils/init/globals.c @@ -81,8 +81,6 @@ char postgres_exec_path[MAXPGPATH]; /* full path to backend */ /* note: currently this is not valid in backend processes */ #endif -BackendId MyBackendId = InvalidBackendId; - BackendId ParallelLeaderBackendId = InvalidBackendId; Oid MyDatabaseId = InvalidOid; diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c index 78bc64671e..7f5b8e12ee 100644 --- a/src/backend/utils/init/postinit.c +++ b/src/backend/utils/init/postinit.c @@ -592,15 +592,10 @@ InitPostgres(const char *in_dbname, Oid dboid, const char *username, * * Sets up MyBackendId, a unique backend identifier. */ - MyBackendId = InvalidBackendId; - SharedInvalBackendInit(false); - if (MyBackendId > MaxBackends || MyBackendId <= 0) - elog(FATAL, "bad backend ID: %d", MyBackendId); - /* Now that we have a BackendId, we can participate in ProcSignal */ - ProcSignalInit(MyBackendId); + ProcSignalInit(); /* * Also set up timeout handlers needed for backend operation. We need diff --git a/src/backend/utils/time/snapmgr.c b/src/backend/utils/time/snapmgr.c index 5001efdf7a..df8476d177 100644 --- a/src/backend/utils/time/snapmgr.c +++ b/src/backend/utils/time/snapmgr.c @@ -1173,7 +1173,7 @@ ExportSnapshot(Snapshot snapshot) * inside the transaction from 1. */ snprintf(path, sizeof(path), SNAPSHOT_EXPORT_DIR "/%08X-%08X-%d", - MyProc->backendId, MyProc->lxid, list_length(exportedSnapshots) + 1); + MyProc->pgprocno, MyProc->lxid, list_length(exportedSnapshots) + 1); /* * Copy the snapshot into TopTransactionContext, add it to the @@ -1200,7 +1200,7 @@ ExportSnapshot(Snapshot snapshot) */ initStringInfo(&buf); - appendStringInfo(&buf, "vxid:%d/%u\n", MyProc->backendId, MyProc->lxid); + appendStringInfo(&buf, "vxid:%d/%u\n", MyProc->pgprocno, MyProc->lxid); appendStringInfo(&buf, "pid:%d\n", MyProcPid); appendStringInfo(&buf, "dbid:%u\n", MyDatabaseId); appendStringInfo(&buf, "iso:%d\n", XactIsoLevel); diff --git a/src/include/storage/backendid.h b/src/include/storage/backendid.h index 7aa3936899..3772e2b4a2 100644 --- a/src/include/storage/backendid.h +++ b/src/include/storage/backendid.h @@ -20,7 +20,8 @@ */ typedef int BackendId; /* unique currently active backend identifier */ -#define InvalidBackendId (-1) +#define INVALID_PGPROCNO PG_INT32_MAX +#define InvalidBackendId INVALID_PGPROCNO extern PGDLLIMPORT BackendId MyBackendId; /* backend id of this backend */ diff --git a/src/include/storage/lock.h b/src/include/storage/lock.h index 9b2a421c32..13b9352704 100644 --- a/src/include/storage/lock.h +++ b/src/include/storage/lock.h @@ -62,7 +62,7 @@ extern bool Debug_deadlocks; */ typedef struct { - BackendId backendId; /* backendId from PGPROC */ + BackendId backendId; /* pgprocno from PGPROC */ LocalTransactionId localTransactionId; /* lxid from PGPROC */ } VirtualTransactionId; @@ -79,7 +79,7 @@ typedef struct ((vxid).backendId = InvalidBackendId, \ (vxid).localTransactionId = InvalidLocalTransactionId) #define GET_VXID_FROM_PGPROC(vxid, proc) \ - ((vxid).backendId = (proc).backendId, \ + ((vxid).backendId = (proc).pgprocno, \ (vxid).localTransactionId = (proc).lxid) /* MAX_LOCKMODES cannot be larger than the # of bits in LOCKMASK */ @@ -445,7 +445,7 @@ typedef struct LockInstanceData LOCKTAG locktag; /* tag for locked object */ LOCKMASK holdMask; /* locks held by this PGPROC */ LOCKMODE waitLockMode; /* lock awaited by this PGPROC, if any */ - BackendId backend; /* backend ID of this PGPROC */ + BackendId backend; /* pgprocno of this PGPROC */ LocalTransactionId lxid; /* local transaction ID of this PGPROC */ TimestampTz waitStart; /* time at which this PGPROC started waiting * for lock */ diff --git a/src/include/storage/proc.h b/src/include/storage/proc.h index be67d8a861..2f3d9cfb17 100644 --- a/src/include/storage/proc.h +++ b/src/include/storage/proc.h @@ -17,6 +17,7 @@ #include "access/clog.h" #include "access/xlogdefs.h" #include "lib/ilist.h" +#include "storage/backendid.h" #include "storage/latch.h" #include "storage/lock.h" #include "storage/pg_sema.h" @@ -60,6 +61,7 @@ struct XidCache #define PROC_VACUUM_FOR_WRAPAROUND 0x08 /* set by autovac only */ #define PROC_IN_LOGICAL_DECODING 0x10 /* currently doing logical * decoding outside xact */ +#define PROC_IS_ACTIVE 0x20 /* This process is active */ /* flags reset at EOXact */ #define PROC_VACUUM_STATE_MASK \ @@ -73,11 +75,7 @@ struct XidCache */ #define FP_LOCK_SLOTS_PER_BACKEND 16 -/* - * An invalid pgprocno. Must be larger than the maximum number of PGPROC - * structures we could possibly have. See comments for MAX_BACKENDS. - */ -#define INVALID_PGPROCNO PG_INT32_MAX +BackendId MyBackendId; typedef enum { @@ -150,7 +148,6 @@ struct PGPROC int pgprocno; /* These fields are zero while a backend is still starting up: */ - BackendId backendId; /* This backend's backend ID (if assigned) */ Oid databaseId; /* OID of database this backend is using */ Oid roleId; /* OID of role using this backend */ @@ -418,4 +415,6 @@ extern PGPROC *AuxiliaryPidGetProc(int pid); extern void BecomeLockGroupLeader(void); extern bool BecomeLockGroupMember(PGPROC *leader, int pid); +extern PGPROC *GetProcIfAlive(BackendId backend); + #endif /* _PROC_H_ */ diff --git a/src/include/storage/procsignal.h b/src/include/storage/procsignal.h index eec186be2e..34a7fff271 100644 --- a/src/include/storage/procsignal.h +++ b/src/include/storage/procsignal.h @@ -63,9 +63,9 @@ typedef enum extern Size ProcSignalShmemSize(void); extern void ProcSignalShmemInit(void); -extern void ProcSignalInit(int pss_idx); +extern void ProcSignalInit(void); extern int SendProcSignal(pid_t pid, ProcSignalReason reason, - BackendId backendId); + int pgprocno); extern uint64 EmitProcSignalBarrier(ProcSignalBarrierType type); extern void WaitForProcSignalBarrier(uint64 generation); diff --git a/src/include/storage/smgr.h b/src/include/storage/smgr.h index a6fbf7b6a6..7c1063f9f9 100644 --- a/src/include/storage/smgr.h +++ b/src/include/storage/smgr.h @@ -78,7 +78,7 @@ typedef SMgrRelationData *SMgrRelation; RelFileNodeBackendIsTemp((smgr)->smgr_rnode) extern void smgrinit(void); -extern SMgrRelation smgropen(RelFileNode rnode, BackendId backend); +extern SMgrRelation smgropen(RelFileNode rnode, int backend); extern bool smgrexists(SMgrRelation reln, ForkNumber forknum); extern void smgrsetowner(SMgrRelation *owner, SMgrRelation reln); extern void smgrclearowner(SMgrRelation *owner, SMgrRelation reln); diff --git a/src/include/utils/rel.h b/src/include/utils/rel.h index b4faa1c123..1ef0571201 100644 --- a/src/include/utils/rel.h +++ b/src/include/utils/rel.h @@ -56,7 +56,7 @@ typedef struct RelationData RelFileNode rd_node; /* relation physical identifier */ SMgrRelation rd_smgr; /* cached file handle, or NULL */ int rd_refcnt; /* reference count */ - BackendId rd_backend; /* owning backend id, if temporary relation */ + int rd_backend; /* owning backend id, if temporary relation */ bool rd_islocaltemp; /* rel is a temp rel of this session */ bool rd_isnailed; /* rel is nailed in cache */ bool rd_isvalid; /* relcache entry is valid */ -- 2.27.0 From 905ec70637191636e1e997e9a42e867a3521f768 Mon Sep 17 00:00:00 2001 From: Kyotaro Horiguchi <horikyota.ntt@gmail.com> Date: Fri, 15 Oct 2021 13:15:13 +0900 Subject: [PATCH v2 3/5] Remove O(n) behavior of SIInsertDataEntries SIInsertDataEntries ran a loop over a certain range of procState array at every insertion. Get rid of the behavior so that we won't be annoyed by performance drag that may happen when procState array gets sparse. This commit removes hasMessage from ProcState. Instead the function uses maxMsgNum to check for new messages. --- src/backend/storage/ipc/sinvaladt.c | 63 +++++++++-------------------- 1 file changed, 20 insertions(+), 43 deletions(-) diff --git a/src/backend/storage/ipc/sinvaladt.c b/src/backend/storage/ipc/sinvaladt.c index a90e9920e7..ee3a3accfd 100644 --- a/src/backend/storage/ipc/sinvaladt.c +++ b/src/backend/storage/ipc/sinvaladt.c @@ -143,7 +143,6 @@ typedef struct ProcState int nextMsgNum; /* next message number to read */ bool resetState; /* backend needs to reset its state */ bool signaled; /* backend has been sent catchup signal */ - bool hasMessages; /* backend has unread messages */ /* * Backend only sends invalidations, never receives them. This only makes @@ -244,7 +243,6 @@ CreateSharedInvalidationState(void) shmInvalBuffer->procState[i].nextMsgNum = 0; /* meaningless */ shmInvalBuffer->procState[i].resetState = false; shmInvalBuffer->procState[i].signaled = false; - shmInvalBuffer->procState[i].hasMessages = false; shmInvalBuffer->procState[i].nextLXID = InvalidLocalTransactionId; } } @@ -309,7 +307,6 @@ SharedInvalBackendInit(bool sendOnly) stateP->nextMsgNum = segP->maxMsgNum; stateP->resetState = false; stateP->signaled = false; - stateP->hasMessages = false; stateP->sendOnly = sendOnly; LWLockRelease(SInvalWriteLock); @@ -417,7 +414,6 @@ SIInsertDataEntries(const SharedInvalidationMessage *data, int n) int nthistime = Min(n, WRITE_QUANTUM); int numMsgs; int max; - int i; n -= nthistime; @@ -450,24 +446,13 @@ SIInsertDataEntries(const SharedInvalidationMessage *data, int n) max++; } - /* Update current value of maxMsgNum using spinlock */ - SpinLockAcquire(&segP->msgnumLock); + /* + * Update current value of maxMsgNum without taking locks. Make sure + * the inserted messages habve been flushed to main memory before the + * update of the shared variable, + */ + pg_memory_barrier(); segP->maxMsgNum = max; - SpinLockRelease(&segP->msgnumLock); - - /* - * Now that the maxMsgNum change is globally visible, we give everyone - * a swift kick to make sure they read the newly added messages. - * Releasing SInvalWriteLock will enforce a full memory barrier, so - * these (unlocked) changes will be committed to memory before we exit - * the function. - */ - for (i = 0; i < segP->lastBackend; i++) - { - ProcState *stateP = &segP->procState[i]; - - stateP->hasMessages = true; - } LWLockRelease(SInvalWriteLock); } @@ -523,27 +508,24 @@ SIGetDataEntries(SharedInvalidationMessage *data, int datasize) * invalidations, any such occurrence is not much different than if the * invalidation had arrived slightly later in the first place. */ - if (!stateP->hasMessages) + max = segP->maxMsgNum; + + if (stateP->nextMsgNum >= max) return 0; LWLockAcquire(SInvalReadLock, LW_SHARED); - /* - * We must reset hasMessages before determining how many messages we're - * going to read. That way, if new messages arrive after we have - * determined how many we're reading, the flag will get reset and we'll - * notice those messages part-way through. - * - * Note that, if we don't end up reading all of the messages, we had - * better be certain to reset this flag before exiting! - */ - stateP->hasMessages = false; - - /* Fetch current value of maxMsgNum using spinlock */ - SpinLockAcquire(&segP->msgnumLock); - max = segP->maxMsgNum; - SpinLockRelease(&segP->msgnumLock); - + if (stateP->nextMsgNum < max) + { + /* + * nextMsgNum has been rewinded before we acquired the lock. Recheck + * with maxMsgNum again. + */ + max = segP->maxMsgNum; + if (stateP->nextMsgNum >= max) + return 0; + } + if (stateP->resetState) { /* @@ -576,14 +558,9 @@ SIGetDataEntries(SharedInvalidationMessage *data, int datasize) /* * If we have caught up completely, reset our "signaled" flag so that * we'll get another signal if we fall behind again. - * - * If we haven't caught up completely, reset the hasMessages flag so that - * we see the remaining messages next time. */ if (stateP->nextMsgNum >= max) stateP->signaled = false; - else - stateP->hasMessages = true; LWLockRelease(SInvalReadLock); return n; -- 2.27.0 From 6338a5397e9103609ba9e842e2390ffab4b9aa5c Mon Sep 17 00:00:00 2001 From: Kyotaro Horiguchi <horikyota.ntt@gmail.com> Date: Fri, 15 Oct 2021 14:09:30 +0900 Subject: [PATCH v2 4/5] Get rid of two O(1) behavior in procarray. GetVirtualXIDsDelayingChkpt had O(N^2) behavior and SignalVirtualTransaction had O(N) behavior. Since the old BackendId has gone they can be reduced to O(N) and O(1) respectively. --- src/backend/storage/ipc/procarray.c | 74 ++++++++++------------------- 1 file changed, 26 insertions(+), 48 deletions(-) diff --git a/src/backend/storage/ipc/procarray.c b/src/backend/storage/ipc/procarray.c index 100c0dae8c..70ce58b3fc 100644 --- a/src/backend/storage/ipc/procarray.c +++ b/src/backend/storage/ipc/procarray.c @@ -3079,41 +3079,24 @@ GetVirtualXIDsDelayingChkpt(int *nvxids) * * This is used with the results of GetVirtualXIDsDelayingChkpt to see if any * of the specified VXIDs are still in critical sections of code. - * - * Note: this is O(N^2) in the number of vxacts that are/were delaying, but - * those numbers should be small enough for it not to be a problem. */ bool HaveVirtualXIDsDelayingChkpt(VirtualTransactionId *vxids, int nvxids) { bool result = false; - ProcArrayStruct *arrayP = procArray; int index; LWLockAcquire(ProcArrayLock, LW_SHARED); - for (index = 0; index < arrayP->numProcs; index++) + for (index = 0; index < nvxids; index++) { - int pgprocno = arrayP->pgprocnos[index]; - PGPROC *proc = &allProcs[pgprocno]; - VirtualTransactionId vxid; + VirtualTransactionId *vxid = &vxids[index]; + PGPROC *proc = &allProcs[vxid->backendId]; - GET_VXID_FROM_PGPROC(vxid, *proc); - - if (proc->delayChkpt && VirtualTransactionIdIsValid(vxid)) + if (proc->delayChkpt && vxid->localTransactionId == proc->lxid) { - int i; - - for (i = 0; i < nvxids; i++) - { - if (VirtualTransactionIdEquals(vxid, vxids[i])) - { - result = true; - break; - } - } - if (result) - break; + result = true; + break; } } @@ -3429,35 +3412,30 @@ pid_t SignalVirtualTransaction(VirtualTransactionId vxid, ProcSignalReason sigmode, bool conflictPending) { - ProcArrayStruct *arrayP = procArray; - int index; - pid_t pid = 0; + PGPROC *proc; + pid_t pid; + VirtualTransactionId procvxid PG_USED_FOR_ASSERTS_ONLY; LWLockAcquire(ProcArrayLock, LW_SHARED); - for (index = 0; index < arrayP->numProcs; index++) + proc = &allProcs[vxid.backendId]; + +#ifdef USE_ASSERT_CHECKING + GET_VXID_FROM_PGPROC(procvxid, *proc); + Assert (procvxid.backendId == vxid.backendId && + procvxid.localTransactionId == vxid.localTransactionId); +#endif + + pid = proc->pid; + proc->recoveryConflictPending = conflictPending; + + if (pid != 0) { - int pgprocno = arrayP->pgprocnos[index]; - PGPROC *proc = &allProcs[pgprocno]; - VirtualTransactionId procvxid; - - GET_VXID_FROM_PGPROC(procvxid, *proc); - - if (procvxid.backendId == vxid.backendId && - procvxid.localTransactionId == vxid.localTransactionId) - { - proc->recoveryConflictPending = conflictPending; - pid = proc->pid; - if (pid != 0) - { - /* - * Kill the pid if it's still here. If not, that's what we - * wanted so ignore any errors. - */ - (void) SendProcSignal(pid, sigmode, proc->pgprocno); - } - break; - } + /* + * Kill the pid if it's still here. If not, that's what we + * wanted so ignore any errors. + */ + (void) SendProcSignal(pid, sigmode, proc->pgprocno); } LWLockRelease(ProcArrayLock); -- 2.27.0 From db5bdd8e1a350cf77b77884ec445b8f0f18f135a Mon Sep 17 00:00:00 2001 From: Kyotaro Horiguchi <horikyota.ntt@gmail.com> Date: Fri, 15 Oct 2021 14:23:52 +0900 Subject: [PATCH v2 5/5] Get rid of the old BackendId at all Since we have got rid of the annoyging O(N) behavior in SIInsertDataEntries, sinvaladt.c can say good-by to the old packed Backend ID. Still CleanupInvalidationState and SICleanupQueue has O(N) behavior but they are executed rarely or ends in a short time in the most cases. --- src/backend/storage/ipc/sinvaladt.c | 67 ++++++----------------------- 1 file changed, 14 insertions(+), 53 deletions(-) diff --git a/src/backend/storage/ipc/sinvaladt.c b/src/backend/storage/ipc/sinvaladt.c index ee3a3accfd..d56c77400f 100644 --- a/src/backend/storage/ipc/sinvaladt.c +++ b/src/backend/storage/ipc/sinvaladt.c @@ -193,8 +193,6 @@ static LocalTransactionId nextLocalTransactionId; static void CleanupInvalidationState(int status, Datum arg); -static int siindex; - /* * SInvalShmemSize --- return shared-memory space needed */ @@ -254,7 +252,6 @@ CreateSharedInvalidationState(void) void SharedInvalBackendInit(bool sendOnly) { - int index; ProcState *stateP = NULL; SISeg *segP = shmInvalBuffer; @@ -265,38 +262,15 @@ SharedInvalBackendInit(bool sendOnly) */ LWLockAcquire(SInvalWriteLock, LW_EXCLUSIVE); - /* Look for a free entry in the procState array */ - for (index = 0; index < segP->lastBackend; index++) - { - if (segP->procState[index].procPid == 0) /* inactive slot? */ - { - stateP = &segP->procState[index]; - break; - } - } + Assert (MyBackendId < MaxBackends); + stateP = &segP->procState[MyBackendId]; - if (stateP == NULL) - { - if (segP->lastBackend < segP->maxBackends) - { - stateP = &segP->procState[segP->lastBackend]; - Assert(stateP->procPid == 0); - segP->lastBackend++; - } - else - { - /* - * out of procState slots: MaxBackends exceeded -- report normally - */ - siindex = -1; - LWLockRelease(SInvalWriteLock); - ereport(FATAL, - (errcode(ERRCODE_TOO_MANY_CONNECTIONS), - errmsg("sorry, too many clients already"))); - } - } + /* this entry should be free */ + Assert (stateP->procPid == 0); - siindex = (stateP - &segP->procState[0]); + /* adjust lastBackend if needed */ + if (segP->lastBackend < MyBackendId) + segP->lastBackend = MyBackendId; /* Fetch next local transaction ID into local memory */ nextLocalTransactionId = stateP->nextLXID; @@ -313,8 +287,6 @@ SharedInvalBackendInit(bool sendOnly) /* register exit routine to mark my entry inactive at exit */ on_shmem_exit(CleanupInvalidationState, PointerGetDatum(segP)); - - elog(DEBUG4, "my SI slot index is %d", siindex); } /* @@ -336,7 +308,7 @@ CleanupInvalidationState(int status, Datum arg) LWLockAcquire(SInvalWriteLock, LW_EXCLUSIVE); - stateP = &segP->procState[siindex]; + stateP = &segP->procState[MyBackendId]; /* Update next local transaction ID for next holder of this backendID */ stateP->nextLXID = nextLocalTransactionId; @@ -349,7 +321,7 @@ CleanupInvalidationState(int status, Datum arg) stateP->signaled = false; /* Recompute index of last active backend */ - for (i = segP->lastBackend; i > 0; i--) + for (i = segP->lastBackend; i >= 0; i--) { if (segP->procState[i - 1].procPid != 0) break; @@ -368,27 +340,16 @@ CleanupInvalidationState(int status, Datum arg) void BackendIdGetTransactionIds(int backendID, TransactionId *xid, TransactionId *xmin) { - SISeg *segP = shmInvalBuffer; - *xid = InvalidTransactionId; *xmin = InvalidTransactionId; - /* Need to lock out additions/removals of backends */ - LWLockAcquire(SInvalWriteLock, LW_SHARED); - - if (backendID > 0 && backendID <= segP->lastBackend) + if (backendID >= 0 && backendID < MaxBackends) { - ProcState *stateP = &segP->procState[backendID - 1]; - PGPROC *proc = stateP->proc; + PGPROC *proc = &ProcGlobal->allProcs[backendID]; - if (proc != NULL) - { - *xid = proc->xid; - *xmin = proc->xmin; - } + *xid = proc->xid; + *xmin = proc->xmin; } - - LWLockRelease(SInvalWriteLock); } /* @@ -495,7 +456,7 @@ SIGetDataEntries(SharedInvalidationMessage *data, int datasize) int n; segP = shmInvalBuffer; - stateP = &segP->procState[siindex]; + stateP = &segP->procState[MyBackendId]; /* * Before starting to take locks, do a quick, unlocked test to see whether -- 2.27.0
pgsql-hackers by date: