Process initialization labyrinth

From: Andres Freund <andres(at)anarazel(dot)de>
To: pgsql-hackers(at)postgresql(dot)org
Subject: Process initialization labyrinth
Date: 2021-04-02 00:22:40
Message-ID: 20210402002240.56cuz3uo3alnqwae@alap3.anarazel.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

While working on the shared memory stats patch I (not for the first
time), issues with our process initialization.

The concrete issue was that I noticed that some stats early in startup
weren't processed correctly - the stats system wasn't initialized yet. I
consequently added assertions ensuring that we don't try to report stats
before that. Which blew up.

Even in master we report stats well before the pgstat_initialize()
call. E.g. in autovac workers:
/*
* Report autovac startup to the stats collector. We deliberately do
* this before InitPostgres, so that the last_autovac_time will get
* updated even if the connection attempt fails. This is to prevent
* autovac from getting "stuck" repeatedly selecting an unopenable
* database, rather than making any progress on stuff it can connect
* to.
*/

That previously just didn't cause a problem, because we didn't really
need pgstat_initialize() to have happened for stats reporting to work.

In the shared memory stats patch there's no dependency on
pgstat_initialize() knowing MyBackendId anymore (broken out to a
separate function). So I tried moving the stats initialization to
somewhere earlier.

There currently is simply no way of doing that that doesn't cause
duplication, or weird conditions. We can't do it in:

- InitProcess()/InitAuxiliaryProcess(),
CreateSharedMemoryAndSemaphores() hasn't yet run in EXEC_BACKEND
- below CreateSharedMemoryAndSemaphores(), as that isn't called for each
backend in !EXEC_BACKEND
- InitPostgres(), because autovac workers report stats before that
- BaseInit(), because it's called before we have a PROC iff !EXEC_BACKEND
- ...

I have now worked around this by generous application of ugly, but I
think we really need to do something about this mazy mess. There's just
an insane amount of duplication, and it's too complicated to remember
more than a few minutes.

I really would like to not see things like

/*
* Create a per-backend PGPROC struct in shared memory, except in the
* EXEC_BACKEND case where this was done in SubPostmasterMain. We must do
* this before we can use LWLocks (and in the EXEC_BACKEND case we already
* had to do some stuff with LWLocks).
*/
#ifdef EXEC_BACKEND
if (!IsUnderPostmaster)
InitProcess();
#else
InitProcess();
#endif

Similarly, codeflow like bootstrap.c being involved in bog standard
stuff like starting up wal writer etc, is just pointlessly
confusing. Note that bootstrap itself does *not* go through
AuxiliaryProcessMain(), and thus has yet another set of initialization
needs.

Greetings,

Andres Freund

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Kohei KaiGai 2021-04-02 00:37:50 Re: TRUNCATE on foreign table
Previous Message Jacob Champion 2021-04-02 00:03:21 Re: Proposal: Save user's original authenticated identity for logging