From b406478b87e2234c0be4ca4105eee3bb466a646b Mon Sep 17 00:00:00 2001 From: Andres Freund Date: Thu, 5 Aug 2021 14:37:09 -0700 Subject: [PATCH] process startup: Always call Init[Auxiliary]Process() before BaseInit(). For EXEC_BACKEND InitProcess()/InitAuxiliaryProcess() needs to have been called well before we call BaseInit(), as SubPostmasterMain() needs LWLocks to work. Having the order of initialization differ between platforms makes it unnecessarily hard to understand the system and to add initialization points for new subsystems without a lot of duplication. To be able to change the order, BaseInit() cannot trigger CreateSharedMemoryAndSemaphores() anymore - obviously that needs to have happened before we can call InitProcess(). It seems cleaner to create shared memory explicitly in single user/bootstrap mode anyway. After this change the separation of bufmgr initialization into InitBufferPoolAccess() / InitBufferPoolBackend() is not meaningful anymore so the latter is removed. Author: Andres Freund Reviewed-By: Kyotaro Horiguchi Discussion: https://postgr.es/m/20210802164124.ufo5buo4apl6yuvs@alap3.anarazel.de --- src/backend/bootstrap/bootstrap.c | 18 +++++++------- src/backend/postmaster/autovacuum.c | 12 +++++----- src/backend/postmaster/auxprocess.c | 7 ++---- src/backend/postmaster/bgworker.c | 16 ++++++------- src/backend/storage/buffer/bufmgr.c | 23 ++++-------------- src/backend/tcop/postgres.c | 8 ++++--- src/backend/utils/init/postinit.c | 37 +++-------------------------- src/include/storage/bufmgr.h | 1 - 8 files changed, 39 insertions(+), 83 deletions(-) diff --git a/src/backend/bootstrap/bootstrap.c b/src/backend/bootstrap/bootstrap.c index 2e2f76a4716..3416802811b 100644 --- a/src/backend/bootstrap/bootstrap.c +++ b/src/backend/bootstrap/bootstrap.c @@ -178,8 +178,8 @@ static IndexList *ILHead = NULL; /* * In shared memory checker mode, all we really want to do is create shared * memory and semaphores (just to prove we can do it with the current GUC - * settings). Since, in fact, that was already done by BaseInit(), - * we have nothing more to do here. + * settings). Since, in fact, that was already done by + * CreateSharedMemoryAndSemaphores(), we have nothing more to do here. */ static void CheckerModeMain(void) @@ -324,7 +324,7 @@ BootstrapModeMain(int argc, char *argv[], bool check_only) InitializeMaxBackends(); - BaseInit(); + CreateSharedMemoryAndSemaphores(); /* * XXX: It might make sense to move this into its own function at some @@ -338,6 +338,13 @@ BootstrapModeMain(int argc, char *argv[], bool check_only) abort(); } + /* + * Do backend-like initialization for bootstrap mode + */ + InitProcess(); + + BaseInit(); + bootstrap_signals(); BootStrapXLOG(); @@ -348,11 +355,6 @@ BootstrapModeMain(int argc, char *argv[], bool check_only) if (pg_link_canary_is_frontend()) elog(ERROR, "backend is incorrectly linked to frontend functions"); - /* - * Do backend-like initialization for bootstrap mode - */ - InitProcess(); - InitPostgres(NULL, InvalidOid, NULL, InvalidOid, NULL, false); /* Initialize stuff for bootstrap-file processing */ diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c index 912ef9cb54c..59d348b062f 100644 --- a/src/backend/postmaster/autovacuum.c +++ b/src/backend/postmaster/autovacuum.c @@ -469,9 +469,6 @@ AutoVacLauncherMain(int argc, char *argv[]) pqsignal(SIGFPE, FloatExceptionHandler); pqsignal(SIGCHLD, SIG_DFL); - /* Early initialization */ - BaseInit(); - /* * Create a per-backend PGPROC struct in shared memory, except in the * EXEC_BACKEND case where this was done in SubPostmasterMain. We must do @@ -482,6 +479,9 @@ AutoVacLauncherMain(int argc, char *argv[]) InitProcess(); #endif + /* Early initialization */ + BaseInit(); + InitPostgres(NULL, InvalidOid, NULL, InvalidOid, NULL, false); SetProcessingMode(NormalProcessing); @@ -1547,9 +1547,6 @@ AutoVacWorkerMain(int argc, char *argv[]) pqsignal(SIGFPE, FloatExceptionHandler); pqsignal(SIGCHLD, SIG_DFL); - /* Early initialization */ - BaseInit(); - /* * Create a per-backend PGPROC struct in shared memory, except in the * EXEC_BACKEND case where this was done in SubPostmasterMain. We must do @@ -1560,6 +1557,9 @@ AutoVacWorkerMain(int argc, char *argv[]) InitProcess(); #endif + /* Early initialization */ + BaseInit(); + /* * If an exception is encountered, processing resumes here. * diff --git a/src/backend/postmaster/auxprocess.c b/src/backend/postmaster/auxprocess.c index 196ee647cfe..16d87e91402 100644 --- a/src/backend/postmaster/auxprocess.c +++ b/src/backend/postmaster/auxprocess.c @@ -90,8 +90,6 @@ AuxiliaryProcessMain(AuxProcType auxtype) SetProcessingMode(BootstrapProcessing); IgnoreSystemIndexes = true; - BaseInit(); - /* * As an auxiliary process, we aren't going to do the full InitPostgres * pushups, but there are a couple of things that need to get lit up even @@ -106,6 +104,8 @@ AuxiliaryProcessMain(AuxProcType auxtype) InitAuxiliaryProcess(); #endif + BaseInit(); + /* * Assign the ProcSignalSlot for an auxiliary process. Since it doesn't * have a BackendId, the slot is statically allocated based on the @@ -118,9 +118,6 @@ AuxiliaryProcessMain(AuxProcType auxtype) */ ProcSignalInit(MaxBackends + MyAuxProcType + 1); - /* finish setting up bufmgr.c */ - InitBufferPoolBackend(); - /* * Auxiliary processes don't run transactions, but they may need a * resource owner anyway to manage buffer pins acquired outside diff --git a/src/backend/postmaster/bgworker.c b/src/backend/postmaster/bgworker.c index c40410d73ea..11c4ceddbf6 100644 --- a/src/backend/postmaster/bgworker.c +++ b/src/backend/postmaster/bgworker.c @@ -837,14 +837,6 @@ StartBackgroundWorker(void) */ if (worker->bgw_flags & BGWORKER_SHMEM_ACCESS) { - /* - * Early initialization. Some of this could be useful even for - * background workers that aren't using shared memory, but they can - * call the individual startup routines for those subsystems if - * needed. - */ - BaseInit(); - /* * Create a per-backend PGPROC struct in shared memory, except in the * EXEC_BACKEND case where this was done in SubPostmasterMain. We must @@ -854,6 +846,14 @@ StartBackgroundWorker(void) #ifndef EXEC_BACKEND InitProcess(); #endif + + /* + * Early initialization. Some of this could be useful even for + * background workers that aren't using shared memory, but they can + * call the individual startup routines for those subsystems if + * needed. + */ + BaseInit(); } /* diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c index 77685bdde28..3b485de067f 100644 --- a/src/backend/storage/buffer/bufmgr.c +++ b/src/backend/storage/buffer/bufmgr.c @@ -2582,11 +2582,6 @@ AtEOXact_Buffers(bool isCommit) * This is called during backend startup (whether standalone or under the * postmaster). It sets up for this backend's access to the already-existing * buffer pool. - * - * NB: this is called before InitProcess(), so we do not have a PGPROC and - * cannot do LWLockAcquire; hence we can't actually access stuff in - * shared memory yet. We are only initializing local data here. - * (See also InitBufferPoolBackend) */ void InitBufferPoolAccess(void) @@ -2600,20 +2595,12 @@ InitBufferPoolAccess(void) PrivateRefCountHash = hash_create("PrivateRefCount", 100, &hash_ctl, HASH_ELEM | HASH_BLOBS); -} -/* - * InitBufferPoolBackend --- second-stage initialization of a new backend - * - * This is called after we have acquired a PGPROC and so can safely get - * LWLocks. We don't currently need to do anything at this stage ... - * except register a shmem-exit callback. AtProcExit_Buffers needs LWLock - * access, and thereby has to be called at the corresponding phase of - * backend shutdown. - */ -void -InitBufferPoolBackend(void) -{ + /* + * AtProcExit_Buffers needs LWLock access, and thereby has to be called at + * the corresponding phase of backend shutdown. + */ + Assert(MyProc != NULL); on_shmem_exit(AtProcExit_Buffers, 0); } diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c index 530caa520b8..58b5960e27d 100644 --- a/src/backend/tcop/postgres.c +++ b/src/backend/tcop/postgres.c @@ -4050,10 +4050,9 @@ PostgresMain(int argc, char *argv[], /* Initialize MaxBackends (if under postmaster, was done already) */ InitializeMaxBackends(); - } - /* Early initialization */ - BaseInit(); + CreateSharedMemoryAndSemaphores(); + } /* * Create a per-backend PGPROC struct in shared memory, except in the @@ -4068,6 +4067,9 @@ PostgresMain(int argc, char *argv[], InitProcess(); #endif + /* Early initialization */ + BaseInit(); + /* We need to allow SIGINT, etc during the initial transaction */ PG_SETMASK(&UnBlockSig); diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c index 51d1bbef301..420b246fb13 100644 --- a/src/backend/utils/init/postinit.c +++ b/src/backend/utils/init/postinit.c @@ -67,7 +67,6 @@ static HeapTuple GetDatabaseTuple(const char *dbname); static HeapTuple GetDatabaseTupleByOid(Oid dboid); static void PerformAuthentication(Port *port); static void CheckMyDatabase(const char *name, bool am_superuser, bool override_allow_connections); -static void InitCommunication(void); static void ShutdownPostgres(int code, Datum arg); static void StatementTimeoutHandler(void); static void LockTimeoutHandler(void); @@ -417,31 +416,6 @@ CheckMyDatabase(const char *name, bool am_superuser, bool override_allow_connect } - -/* -------------------------------- - * InitCommunication - * - * This routine initializes stuff needed for ipc, locking, etc. - * it should be called something more informative. - * -------------------------------- - */ -static void -InitCommunication(void) -{ - /* - * initialize shared memory and semaphores appropriately. - */ - if (!IsUnderPostmaster) /* postmaster already did this */ - { - /* - * We're running a postgres bootstrap process or a standalone backend, - * so we need to set up shmem. - */ - CreateSharedMemoryAndSemaphores(); - } -} - - /* * pg_split_opts -- split a string of options and append it to an argv array * @@ -536,11 +510,11 @@ InitializeMaxBackends(void) void BaseInit(void) { + Assert(MyProc != NULL); + /* - * Attach to shared memory and semaphores, and initialize our - * input/output/debugging file descriptors. + * Initialize our input/output/debugging file descriptors. */ - InitCommunication(); DebugFileOpen(); /* Do local initialization of file, storage and buffer managers */ @@ -624,11 +598,6 @@ InitPostgres(const char *in_dbname, Oid dboid, const char *username, RegisterTimeout(CLIENT_CONNECTION_CHECK_TIMEOUT, ClientCheckTimeoutHandler); } - /* - * bufmgr needs another initialization call too - */ - InitBufferPoolBackend(); - /* * Initialize local process's access to XLOG. */ diff --git a/src/include/storage/bufmgr.h b/src/include/storage/bufmgr.h index aa64fb42ec4..cfce23ecbc8 100644 --- a/src/include/storage/bufmgr.h +++ b/src/include/storage/bufmgr.h @@ -194,7 +194,6 @@ extern Buffer ReleaseAndReadBuffer(Buffer buffer, Relation relation, extern void InitBufferPool(void); extern void InitBufferPoolAccess(void); -extern void InitBufferPoolBackend(void); extern void AtEOXact_Buffers(bool isCommit); extern void PrintBufferLeakWarning(Buffer buffer); extern void CheckPointBuffers(int flags);