Re: Accommodate startup process in a separate ProcState array slot instead of in MaxBackends slots. - Mailing list pgsql-hackers

From Fujii Masao
Subject Re: Accommodate startup process in a separate ProcState array slot instead of in MaxBackends slots.
Date
Msg-id 94536d78-f3cd-b7ba-c6dc-8e7a56e02e6d@oss.nttdata.com
Whole thread Raw
In response to Re: Accommodate startup process in a separate ProcState array slot instead of in MaxBackends slots.  (Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>)
Responses Re: Accommodate startup process in a separate ProcState array slot instead of in MaxBackends slots.  (Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>)
List pgsql-hackers

On 2021/10/12 15:46, Bharath Rupireddy wrote:
> On Tue, Oct 12, 2021 at 5:37 AM Fujii Masao <masao.fujii@oss.nttdata.com> wrote:
>>
>> On 2021/10/12 4:07, Bharath Rupireddy wrote:
>>> Hi,
>>>
>>> While working on [1], it is found that currently the ProcState array
>>> doesn't have entries for auxiliary processes, it does have entries for
>>> MaxBackends. But the startup process is eating up one slot from
>>> MaxBackends. We need to increase the size of the ProcState array by 1
>>> at least for the startup process. The startup process uses ProcState
>>> slot via InitRecoveryTransactionEnvironment->SharedInvalBackendInit.
>>> The procState array size is initialized to MaxBackends in
>>> SInvalShmemSize.
>>>
>>> The consequence of not fixing this issue is that the database may hit
>>> the error "sorry, too many clients already" soon in
>>> SharedInvalBackendInit.

On second thought, I wonder if this error could not happen in practice. No?
Because autovacuum doesn't work during recovery and the startup process
can safely use the ProcState entry for autovacuum worker process.
Also since the minimal allowed value of autovacuum_max_workers is one,
the ProcState array guarantees to have at least one entry for autovacuum worker.

If this understanding is right, we don't need to enlarge the array and
can just update the comment. I don't strongly oppose to enlarge
the array in the master, but I'm not sure it's worth doing that
in back branches if the issue can cause no actual error.


>>>
>>> Attaching a patch to fix this issue. Thoughts?
>>
>> Thanks for making the patch! LGTM.
>> Barring any objection, I will commit it.
> 
> Thanks for reviewing. I've made a CF entry for this, just to ensure
> the tests on different CF bot server passes(and yes no failures) -
> https://commitfest.postgresql.org/35/3355/

Thanks!

Regards,

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



pgsql-hackers by date:

Previous
From: Andrey Lepikhov
Date:
Subject: Re: Make query ID more portable
Next
From: katouknl
Date:
Subject: Re: [PATCH] Added TRANSFORM FOR for COMMENT tab completion