Re: Inconsistency in startup process's MyBackendId and procsignal array registration with ProcSignalInit() - Mailing list pgsql-hackers

From Bharath Rupireddy
Subject Re: Inconsistency in startup process's MyBackendId and procsignal array registration with ProcSignalInit()
Date
Msg-id CALj2ACW=yqLAXADVE-xXf=9wXxCjUJcB2nbMYksayTrV-UVcCg@mail.gmail.com
Whole thread Raw
In response to Re: Inconsistency in startup process's MyBackendId and procsignal array registration with ProcSignalInit()  (Fujii Masao <masao.fujii@oss.nttdata.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
On Mon, Oct 11, 2021 at 8:59 PM Fujii Masao <masao.fujii@oss.nttdata.com> wrote:
>
> 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?

The patch looks good to me unless I'm missing something badly.

+ Assert(MyBackendId == InvalidBackendId ||
+    MyBackendId <= segP->maxBackends);
+
In the above assertion, we can just do MyBackendId ==
segP->maxBackends, instead of <= as the startup process is the only
process that calls SharedInvalBackendInit with pre-calculated
MyBackendId = MaxBackends + MyAuxProcType + 1; and it will always
occupy the last slot in the procState array.

Otherwise, we could discard defining MyBackendId in auxprocess.c and
define the MyBackendId in the SharedInvalBackendInit itself as this is
the function that defines the MyBackendId for everyone whoever
requires it. I prefer this approach over what's done in PoC patch.

In SharedInvalBackendInit:
Assert(MyBackendId == InvalidBackendId);
/*
* The startup process requires a valid BackendId for the SI message
* buffer and virtual transaction id, so define it here with the value with
* which the procsignal array slot was allocated in AuxiliaryProcessMain.
* All other auxiliary processes don't need it.
*/
if (MyAuxProcType == StartupProcess)
    MyBackendId = MaxBackends + MyAuxProcType + 1;

I think this solution, coupled with the one proposed at [1], should
solve this startup process's inconsistency in MyBackendId, procState
and ProcSignal array slot problems.

[1] - https://www.postgresql.org/message-id/CALj2ACXZ_o7rcOb7-Rs96P0d%3DEi%2Bnvf_WZ-Meky7Vv%2BnQNFYjQ%40mail.gmail.com

Regards,
Bharath Rupireddy.



pgsql-hackers by date:

Previous
From: Kyotaro Horiguchi
Date:
Subject: Re: Allow escape in application_name
Next
From: Kyotaro Horiguchi
Date:
Subject: Re: Inconsistency in startup process's MyBackendId and procsignal array registration with ProcSignalInit()