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
