Re: Inconsistency in startup process's MyBackendId and procsignal array registration with ProcSignalInit()

[Fujii Masao]

On 2021/10/11 19:46, Bharath Rupireddy wrote:

If we do the above, then the problem might arise if somebody calls
SICleanupQueue and wants to signal the startup process, the below code
(from SICleanupQueue) can't get the startup process backend id. So,
the backend id calculation for the startup process can't just be
MaxBackends + MyAuxProcType + 1.
BackendId his_backendId = (needSig - &segP->procState[0]) + 1;

Attached POC patch illustrates what I'm in mind. ISTM this change
doesn't prevent SICleanupQueue() from getting right backend ID
of the startup process. Thought?

It looks like we need to increase the size of the ProcState array by 1
at least (for the startup process). Currently the ProcState array
doesn't have entries for auxiliary processes, it does have entries for
MaxBackends. The startup process is eating up one slot from
MaxBackends. Since we need only an extra ProcState array slot for the
startup process I think we could just extend its size by 1. Instead of
modifying the MaxBackends definition, we can just add 1 (and a comment
saying this 1 is for startup process) to  shmInvalBuffer->maxBackends
in SInvalShmemSize, CreateSharedInvalidationState. IMO, this has to go
in a separate patch and probably in a separate thread. Thoughts?

Agreed.

Regards,

--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION

diff --git a/src/backend/postmaster/auxprocess.c 
b/src/backend/postmaster/auxprocess.c
index 7452f908b2..3f53138e7f 100644
--- a/src/backend/postmaster/auxprocess.c
+++ b/src/backend/postmaster/auxprocess.c
@@ -138,6 +138,7 @@ AuxiliaryProcessMain(AuxProcType auxtype)
        switch (MyAuxProcType)
        {
                case StartupProcess:
+                       MyBackendId = MaxBackends + MyAuxProcType + 1;
                        StartupProcessMain();
                        proc_exit(1);
 
diff --git a/src/backend/storage/ipc/sinvaladt.c 
b/src/backend/storage/ipc/sinvaladt.c
index 946bd8e3cb..af7de450fe 100644
--- a/src/backend/storage/ipc/sinvaladt.c
+++ b/src/backend/storage/ipc/sinvaladt.c
@@ -231,7 +231,7 @@ CreateSharedInvalidationState(void)
        shmInvalBuffer->maxMsgNum = 0;
        shmInvalBuffer->nextThreshold = CLEANUP_MIN;
        shmInvalBuffer->lastBackend = 0;
-       shmInvalBuffer->maxBackends = MaxBackends;
+       shmInvalBuffer->maxBackends = MaxBackends + 1;
        SpinLockInit(&shmInvalBuffer->msgnumLock);
 
        /* The buffer[] array is initially all unused, so we need not fill it */
@@ -260,6 +260,9 @@ SharedInvalBackendInit(bool sendOnly)
        ProcState  *stateP = NULL;
        SISeg      *segP = shmInvalBuffer;
 
+       Assert(MyBackendId == InvalidBackendId ||
+                  MyBackendId <= segP->maxBackends);
+
        /*
         * This can run in parallel with read operations, but not with write
         * operations, since SIInsertDataEntries relies on lastBackend to set
@@ -267,15 +270,23 @@ SharedInvalBackendInit(bool sendOnly)
         */
        LWLockAcquire(SInvalWriteLock, LW_EXCLUSIVE);
 
-       /* Look for a free entry in the procState array */
-       for (index = 0; index < segP->lastBackend; index++)
+       if (MyBackendId == InvalidBackendId)
        {
-               if (segP->procState[index].procPid == 0)        /* inactive 
slot? */
+               /* Look for a free entry in the procState array */
+               for (index = 0; index < segP->lastBackend; index++)
                {
-                       stateP = &segP->procState[index];
-                       break;
+                       if (segP->procState[index].procPid == 0)        /* 
inactive slot? */
+                       {
+                               stateP = &segP->procState[index];
+                               break;
+                       }
                }
        }
+       else
+       {
+               stateP = &segP->procState[MyBackendId - 1];
+               Assert(stateP->procPid == 0);
+       }
 
        if (stateP == NULL)
        {
@@ -298,6 +309,8 @@ SharedInvalBackendInit(bool sendOnly)
                }
        }
 
+       Assert(MyBackendId == InvalidBackendId ||
+                  MyBackendId == (stateP - &segP->procState[0]) + 1);
        MyBackendId = (stateP - &segP->procState[0]) + 1;
 
        /* Advertise assigned backend ID in MyProc */