Re: Set appropriate processing mode for auxiliary processes.

On 10/05/2024 05:58, Xing Guo wrote:
> On Thu, May 9, 2024 at 11:19 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>
>> Heikki Linnakangas <hlinnaka(at)iki(dot)fi> writes:
>>> At first I was sure this was introduced by my refactorings in v17, but
>>> in fact it's been like this forever. I agree that InitProcessing makes
>>> much more sense. The ProcessingMode variable is initialized to
>>> InitProcessing, so I think we can simply remove that line from
>>> AuxiliaryProcessMainCommon(). There are existing
>>> "SetProcessingMode(InitProcessing)" calls in other Main functions too
>>> (AutoVacLauncherMain, BackgroundWorkerMain, etc.), and I think those can
>>> also be removed.
>>
>> This only works if the postmaster can be trusted never to change the
>> variable; else children could inherit some other value via fork().
>> In that connection, it seems a bit scary that postmaster.c contains a
>> couple of calls "SetProcessingMode(NormalProcessing)". It looks like
>> they are in functions that should only be executed by child processes,
>> but should we try to move them somewhere else?
>
> After checking calls to "SetProcessingMode(NormalProcessing)" in the
> postmaster.c, they are used in background worker specific functions
> (BackgroundWorkerInitializeConnectionByOid and
> BackgroundWorkerInitializeConnection). So I think it's a good idea to
> move these functions to bgworker.c. Then, we can get rid of calling
> "SetProcessingMode(NormalProcessing)" in postmaster.c.

Committed these first two patches. Thank you!

> I also noticed that there's an unnecessary call to
> "BackgroundWorkerInitializeConnection" in worker_spi.c (The worker_spi
> launcher has set the dboid correctly).

No, you can call the launcher function with "dboid=0", and it's also 0
in the "static" registration at end of _PG_init(). This causes
regression tests to fail too because of that. So I left out that patch.

--
Heikki Linnakangas
Neon (https://neon.tech)