From 609975ac219d069552c7e574ce4bc500b25ed010 Mon Sep 17 00:00:00 2001 From: Julien Rouhaud Date: Sat, 14 May 2022 14:54:22 +0800 Subject: [PATCH 1/4] Remove _PG_fini(). Postgres hasn't called this function for more than a decade (even before extensions were introduced), and version 15 officially removed it so let's get rid of it too. --- pg_wait_sampling.c | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/pg_wait_sampling.c b/pg_wait_sampling.c index d5a998e..f42528f 100644 --- a/pg_wait_sampling.c +++ b/pg_wait_sampling.c @@ -39,7 +39,6 @@ PG_MODULE_MAGIC; void _PG_init(void); -void _PG_fini(void); /* Global variables */ bool shmem_initialized = false; @@ -359,22 +358,14 @@ _PG_init(void) */ prev_shmem_startup_hook = shmem_startup_hook; shmem_startup_hook = pgws_shmem_startup; + prev_shmem_startup_hook = shmem_startup_hook; + shmem_startup_hook = pgws_shmem_startup; planner_hook_next = planner_hook; planner_hook = pgws_planner_hook; prev_ExecutorEnd = ExecutorEnd_hook; ExecutorEnd_hook = pgws_ExecutorEnd; } -/* - * Module unload callback - */ -void -_PG_fini(void) -{ - /* Uninstall hooks. */ - shmem_startup_hook = prev_shmem_startup_hook; -} - /* * Find PGPROC entry responsible for given pid assuming ProcArrayLock was * already taken. From 47616672ebdec64292690e2e37632255ba34531d Mon Sep 17 00:00:00 2001 From: Julien Rouhaud Date: Sat, 14 May 2022 15:47:14 +0800 Subject: [PATCH 2/4] Wait 1 second before restarting the bgworker. If for some reason the bgworker keeps crashing, this will limit the amount of traces logged when restarting. --- collector.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/collector.c b/collector.c index 2fa80c3..a49f90c 100644 --- a/collector.c +++ b/collector.c @@ -44,7 +44,7 @@ register_wait_collector(void) memset(&worker, 0, sizeof(worker)); worker.bgw_flags = BGWORKER_SHMEM_ACCESS; worker.bgw_start_time = BgWorkerStart_ConsistentState; - worker.bgw_restart_time = 0; + worker.bgw_restart_time = 1; worker.bgw_notify_pid = 0; snprintf(worker.bgw_library_name, BGW_MAXLEN, "pg_wait_sampling"); snprintf(worker.bgw_function_name, BGW_MAXLEN, CppAsString(collector_main)); From 1a8aa648fb478499fdc02285aaba2f8a390833f6 Mon Sep 17 00:00:00 2001 From: Julien Rouhaud Date: Sat, 14 May 2022 15:55:28 +0800 Subject: [PATCH 3/4] Setup SIGUSR1 procsignal_sigusr1_handler handler. This is normally automatically done when a bgworker declares the BGWORKER_BACKEND_DATABASE_CONNECTION flag, but our bgworker doesn't connect to databases, even though it calls InitPostgres, which will still initialize a new bacckend and thus participate to the ProcSignal infrastructure. This wasn't a problem until recently, but now that DROP DATABASE relies on PROCSIG_BARRIER to work on Windows (see commit 4eb2176318d0561846c1f9fb3c68bede799d640f) we need to respond to ProcSignal notifications. --- collector.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/collector.c b/collector.c index a49f90c..bbf2741 100644 --- a/collector.c +++ b/collector.c @@ -339,8 +339,16 @@ collector_main(Datum main_arg) * any equivalent of the backend's command-read loop, where interrupts can * be processed immediately, so make sure ImmediateInterruptOK is turned * off. + * + * We also want to respond to the ProcSignal notifications. This is done + * in the upstream provided procsignal_sigusr1_handler, which is + * automatically used if a bgworker connects to a database. But since our + * worker doesn't connect to any database even though it calls + * InitPostgres, which will still initializze a new backend and thus + * partitipate to the ProcSignal infrastructure. */ pqsignal(SIGTERM, handle_sigterm); + pqsignal(SIGUSR1, procsignal_sigusr1_handler); BackgroundWorkerUnblockSignals(); #if PG_VERSION_NUM >= 110000 @@ -379,6 +387,9 @@ collector_main(Datum main_arg) bool write_history, write_profile; + /* We need an explicit call for at least ProcSignal notifications. */ + CHECK_FOR_INTERRUPTS(); + /* Wait calculate time to next sample for history or profile */ current_ts = GetCurrentTimestamp(); From 5b50e788951caa4cec6606b2b5f39897e983ea05 Mon Sep 17 00:00:00 2001 From: Julien Rouhaud Date: Sat, 14 May 2022 15:59:36 +0800 Subject: [PATCH 4/4] Fix compatibility with pg15 new shmem_request_hook. Hook added upstream in 4f2400cb3f10aa79f99fba680c198237da28dd38. --- pg_wait_sampling.c | 75 +++++++++++++++++++++++++++++++++++----------- 1 file changed, 58 insertions(+), 17 deletions(-) diff --git a/pg_wait_sampling.c b/pg_wait_sampling.c index f42528f..f27a5de 100644 --- a/pg_wait_sampling.c +++ b/pg_wait_sampling.c @@ -58,6 +58,9 @@ shm_mq *recv_mq = NULL; shm_mq_handle *recv_mqh = NULL; LOCKTAG queueTag; +#if PG_VERSION_NUM >= 150000 +static shmem_request_hook_type prev_shmem_request_hook = NULL; +#endif static shmem_startup_hook_type prev_shmem_startup_hook = NULL; static PGPROC * search_proc(int backendPid); static PlannedStmt *pgws_planner_hook(Query *parse, @@ -73,28 +76,40 @@ static void pgws_ExecutorEnd(QueryDesc *queryDesc); * The value has to be in sync with ProcGlobal->allProcCount, initialized in * InitProcGlobal() (proc.c). * - * We calculate the value here as it won't initialized when we need it during - * _PG_init(). - * - * Note that the value returned during _PG_init() might be different from the - * value returned later if some third-party modules change one of the - * underlying GUC. This isn't ideal but can't lead to a crash, as the value - * returned during _PG_init() is only used to ask for additional shmem with - * RequestAddinShmemSpace(), and postgres has an extra 100kB of shmem to - * compensate some small unaccounted usage. So if the value later changes, we - * will allocate and initialize the new (and correct) memory size, which - * will either work thanks for the extra 100kB of shmem, of fail (and prevent - * postgres startup) due to an out of shared memory error. */ static int get_max_procs_count(void) { int count = 0; + /* First, add the maximum number of backends (MaxBackends). */ +#if PG_VERSION_NUM >= 150000 + /* + * On pg15+, we can directly access the MaxBackends variable, as it will + * have already been initialized in shmem_request_hook. + */ + Assert(MaxBackends > 0); + count += MaxBackends; +#else /* - * MaxBackends: bgworkers, autovacuum workers and launcher. + * On older versions, we need to compute MaxBackends: bgworkers, autovacuum + * workers and launcher. * This has to be in sync with the value computed in * InitializeMaxBackends() (postinit.c) + * + * Note that we need to calculate the value as it won't initialized when we + * need it during _PG_init(). + * + * Note also that the value returned during _PG_init() might be different + * from the value returned later if some third-party modules change one of + * the underlying GUC. This isn't ideal but can't lead to a crash, as the + * value returned during _PG_init() is only used to ask for additional + * shmem with RequestAddinShmemSpace(), and postgres has an extra 100kB of + * shmem to compensate some small unaccounted usage. So if the value later + * changes, we will allocate and initialize the new (and correct) memory + * size, which will either work thanks for the extra 100kB of shmem, of + * fail (and prevent postgres startup) due to an out of shared memory + * error. */ count += MaxConnections + autovacuum_max_workers + 1 + max_worker_processes; @@ -105,9 +120,11 @@ get_max_procs_count(void) */ #if PG_VERSION_NUM >= 120000 count += max_wal_senders; -#endif +#endif /* pg 12+ */ +#endif /* pg 15- */ + /* End of MaxBackends calculation. */ - /* AuxiliaryProcs */ + /* Add AuxiliaryProcs */ count += NUM_AUXILIARY_PROCS; return count; @@ -265,6 +282,23 @@ setup_gucs() } } +#if PG_VERSION_NUM >= 150000 +/* + * shmem_request hook: request additional shared memory resources. + * + * If you change code here, don't forget to also report the modifications in + * _PG_init() for pg14 and below. + */ +static void +pgws_shmem_request(void) +{ + if (prev_shmem_request_hook) + prev_shmem_request_hook(); + + RequestAddinShmemSpace(pgws_shmem_size()); +} +#endif + /* * Distribute shared memory. */ @@ -344,20 +378,27 @@ _PG_init(void) if (!process_shared_preload_libraries_in_progress) return; +#if PG_VERSION_NUM < 150000 /* * Request additional shared resources. (These are no-ops if we're not in * the postmaster process.) We'll allocate or attach to the shared * resources in pgws_shmem_startup(). + * + * If you change code here, don't forget to also report the modifications + * in pgsp_shmem_request() for pg15 and later. */ RequestAddinShmemSpace(pgws_shmem_size()); +#endif register_wait_collector(); /* * Install hooks. */ - prev_shmem_startup_hook = shmem_startup_hook; - shmem_startup_hook = pgws_shmem_startup; +#if PG_VERSION_NUM >= 150000 + prev_shmem_request_hook = shmem_request_hook; + shmem_request_hook = pgws_shmem_request; +#endif prev_shmem_startup_hook = shmem_startup_hook; shmem_startup_hook = pgws_shmem_startup; planner_hook_next = planner_hook;