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

On Thu, Oct 14, 2021 at 10:56 AM Fujii Masao
<masao(dot)fujii(at)oss(dot)nttdata(dot)com> wrote:
> 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.

Yes, the issue can't happen. The comment in the SInvalShmemSize,
mentioning about the startup process always having an extra slot
because the autovacuum worker is not active during recovery, looks
okay. But, is it safe to assume that always? Do we have a way to
specify that in the form an Assert(when_i_am_startup_proc &&
autovacuum_not_running) (this looks a bit dirty though)? Instead, we
can just enlarge the array in the master and be confident about the
fact that the startup process always has one dedicated slot.

Regards,
Bharath Rupireddy.