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

From: Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>
To: Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com>
Cc: PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Inconsistency in startup process's MyBackendId and procsignal array registration with ProcSignalInit()
Date: 2021-10-12 07:39:47
Message-ID: CALj2ACW=yqLAXADVE-xXf=9wXxCjUJcB2nbMYksayTrV-UVcCg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Oct 11, 2021 at 8:59 PM Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)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.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Kyotaro Horiguchi 2021-10-12 08:09:44 Re: Inconsistency in startup process's MyBackendId and procsignal array registration with ProcSignalInit()
Previous Message Kyotaro Horiguchi 2021-10-12 06:59:23 Re: Allow escape in application_name