Re: Accommodate startup process in a separate ProcState array slot instead of in MaxBackends slots.

From: Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com>
To: Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>
Cc: PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Accommodate startup process in a separate ProcState array slot instead of in MaxBackends slots.
Date: 2021-10-14 05:26:50
Message-ID: 94536d78-f3cd-b7ba-c6dc-8e7a56e02e6d@oss.nttdata.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 2021/10/12 15:46, Bharath Rupireddy wrote:
> On Tue, Oct 12, 2021 at 5:37 AM Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message katouknl 2021-10-14 05:30:19 Re: [PATCH] Added TRANSFORM FOR for COMMENT tab completion
Previous Message Masahiko Sawada 2021-10-14 05:22:21 Re: Inconsistent behavior of pg_dump/pg_restore on DEFAULT PRIVILEGES