Horton

Re: straightening out backend process startup

Andres Freund <andres@anarazel.de>

View thread View on postgr.es
From: Andres Freund <andres@anarazel.de>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: pgsql-hackers@postgresql.org
Date: 2021-08-02T17:12:55Z
Lists: pgsql-hackers 
Hi,

On 2021-08-02 12:57:36 -0400, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
> > I think there's quite a few different issues around this - here I'm just
> > trying to tackle a few of the most glaring (to me):
> 
> No opinion yet about most of this, but I did want to react to
> 
> > In fact, I think there's a good argument to be made that we should
> > entirely remove the concept of BackendIds and just use pgprocnos. We
> > have a fair number of places like SignalVirtualTransaction() that need
> > to search the procarray just to find the proc to signal based on the
> > BackendId. If we used pgprocno instead, that'd not be needed.
> 
> If I understand what you're suggesting, it'd result in unused slots
> in sinvaladt.c's procState[] array, which could create an annoying
> drag on performance.

Yea, I was looking into that, which is why I don't yet have a patch. I think
it might be reasonable to address this by making pgprocnos more dense. We
right now actually have a kind of maximally bad allocation pattern for
pgprocnos - InitProcGlobal puts them onto the free lists in reverse
order. Which means that ProcArrayAdd() will have to move all procs...


But, as you say:

> However, I think it might be reasonable to switch other things over to
> using pgprocnos, with an eye to eventually making BackendIds private
> to the sinval mechanism.  There's certainly no strong reason why
> sinval's array indexes need to be used as identifiers by other
> modules.

I think it'd entirely be reasonable to switch over at least backend_status.c,
procsignal.c to pgprocnos without doing anything about the density of
allocation. We'd likely would want to do that as independent steps anyway,
even if we were to switch over sinval as well.


Another approach to deal with this could be to simply not do the O(n) work in
SIInsertDataEntries(). It's not obvious that ->hasMessages is actually
necessary - we could atomically read maxMsgNum without acquiring a lock
instead of needing the per-backend ->hasMessages.  I don't the density would
be a relevant factor in SICleanupQueue().

Greetings,

Andres Freund

Commits

  1. process startup: Split single user code out of PostgresMain().

  2. process startup: Do InitProcess() at the same time regardless of EXEC_BACKEND.

  3. process startup: Initialize PgStartTime earlier in single user mode.

  4. pgstat: Schedule per-backend pgstat shutdown via before_shmem_exit().

  5. pgstat: Bring up pgstat in BaseInit() to fix uninitialized use of pgstat by AV.

  6. process startup: Always call Init[Auxiliary]Process() before BaseInit().

  7. process startup: Centralize pgwin32_signal_initialize() calls.

  8. process startup: Remove bootstrap / checker modes from AuxProcType.

  9. process startup: Move AuxiliaryProcessMain into its own file.

  10. process startup: auxprocess: reindent block

  11. process startup: Separate out BootstrapModeMain from AuxiliaryProcessMain.

  12. process startup: Rename postmaster's --forkboot to --forkaux.