Store GUC variables in local process memory to avoid IPC shenanigans

For more context and reasoning see issue #85

Author: Oleg Tselebrovskiy

README.md

@@ -158,9 +158,9 @@ If `pg_wait_sampling.sample_cpu` is set to true then processes that are not
 waiting on anything are also sampled. The wait event columns for such processes
 will be NULL.
 
-These GUCs are allowed to be changed by superuser.  Also, they are placed into
-shared memory.  Thus, they could be changed from any backend and affects worker
-runtime.
+Values of these GUC variables can be changed only in config file or with ALTER SYSTEM.
+Then you need to reload server's configuration (such as with pg_reload_conf function)
+for changes to take effect.
 
 See
 [PostgreSQL documentation](http://www.postgresql.org/docs/devel/static/monitoring-stats.html#WAIT-EVENT-TABLE)

collector.c

@@ -16,6 +16,7 @@
 #include "funcapi.h"
 #include "miscadmin.h"
 #include "postmaster/bgworker.h"
+#include "postmaster/interrupt.h"
 #include "storage/ipc.h"
 #include "storage/procarray.h"
 #include "storage/procsignal.h"
@@ -151,7 +152,7 @@ probe_waits(History *observations, HTAB *profile_hash,
 	TimestampTz	ts = GetCurrentTimestamp();
 
 	/* Realloc waits history if needed */
-	newSize = pgws_collector_hdr->historySize;
+	newSize = pg_wait_sampling_historySize;
 	if (observations->count != newSize)
 		realloc_history(observations, newSize);
 
@@ -170,7 +171,7 @@ probe_waits(History *observations, HTAB *profile_hash,
 		item.pid = proc->pid;
 		item.wait_event_info = proc->wait_event_info;
 
-		if (pgws_collector_hdr->profileQueries)
+		if (pg_wait_sampling_profileQueries)
 			item.queryId = pgws_proc_queryids[i];
 		else
 			item.queryId = 0;
@@ -289,7 +290,7 @@ make_profile_hash()
 	hash_ctl.hash = tag_hash;
 	hash_ctl.hcxt = TopMemoryContext;
 
-	if (pgws_collector_hdr->profileQueries)
+	if (pg_wait_sampling_profileQueries)
 		hash_ctl.keysize = offsetof(ProfileItem, count);
 	else
 		hash_ctl.keysize = offsetof(ProfileItem, queryId);
@@ -346,6 +347,7 @@ pgws_collector_main(Datum main_arg)
 	 * partitipate to the ProcSignal infrastructure.
 	 */
 	pqsignal(SIGTERM, handle_sigterm);
+	pqsignal(SIGHUP, SignalHandlerForConfigReload);
 	pqsignal(SIGUSR1, procsignal_sigusr1_handler);
 	BackgroundWorkerUnblockSignals();
 	InitPostgresCompat(NULL, InvalidOid, NULL, InvalidOid, 0, NULL);
@@ -361,7 +363,7 @@ pgws_collector_main(Datum main_arg)
 	collector_context = AllocSetContextCreate(TopMemoryContext,
 			"pg_wait_sampling context", ALLOCSET_DEFAULT_SIZES);
 	old_context = MemoryContextSwitchTo(collector_context);
-	alloc_history(&observations, pgws_collector_hdr->historySize);
+	alloc_history(&observations, pg_wait_sampling_historySize);
 	MemoryContextSwitchTo(old_context);
 
 	ereport(LOG, (errmsg("pg_wait_sampling collector started")));
@@ -383,21 +385,27 @@ pgws_collector_main(Datum main_arg)
 		/* We need an explicit call for at least ProcSignal notifications. */
 		CHECK_FOR_INTERRUPTS();
 
+		if (ConfigReloadPending)
+		{
+			ConfigReloadPending = false;
+			ProcessConfigFile(PGC_SIGHUP);
+		}
+
 		/* Wait calculate time to next sample for history or profile */
 		current_ts = GetCurrentTimestamp();
 
 		history_diff = millisecs_diff(history_ts, current_ts);
 		profile_diff = millisecs_diff(profile_ts, current_ts);
-		history_period = pgws_collector_hdr->historyPeriod;
-		profile_period = pgws_collector_hdr->profilePeriod;
+		history_period = pg_wait_sampling_historyPeriod;
+		profile_period = pg_wait_sampling_profilePeriod;
 
 		write_history = (history_diff >= (int64)history_period);
 		write_profile = (profile_diff >= (int64)profile_period);
 
 		if (write_history || write_profile)
 		{
 			probe_waits(&observations, profile_hash,
-						write_history, write_profile, pgws_collector_hdr->profilePid);
+						write_history, write_profile, pg_wait_sampling_profilePid);
 
 			if (write_history)
 			{

compat.h

@@ -51,18 +51,4 @@ InitPostgresCompat(const char *in_dbname, Oid dboid,
 #endif
 }
 
-static inline void
-get_guc_variables_compat(struct config_generic ***vars, int *num_vars)
-{
-	Assert(vars != NULL);
-	Assert(num_vars != NULL);
-
-#if PG_VERSION_NUM >= 160000
-	*vars = get_guc_variables(num_vars);
-#else
-	*vars = get_guc_variables();
-	*num_vars = GetNumConfigOptions();
-#endif
-}
-
 #endif

pg_wait_sampling.c

@@ -120,9 +120,17 @@ static const struct config_enum_entry pgws_profile_queries_options[] =
 	{NULL, 		0, false}
 };
 
+/* GUC variables */
+int pg_wait_sampling_historySize = 5000;
+int pg_wait_sampling_historyPeriod = 10;
+int pg_wait_sampling_profilePeriod = 10;
+bool pg_wait_sampling_profilePid = true;
+int pg_wait_sampling_profileQueries = PGWS_PROFILE_QUERIES_TOP;
+bool pg_wait_sampling_sampleCpu = true;
+
 #define pgws_enabled(level) \
-	((pgws_collector_hdr->profileQueries == PGWS_PROFILE_QUERIES_ALL) || \
-	 (pgws_collector_hdr->profileQueries == PGWS_PROFILE_QUERIES_TOP && (level) == 0))
+	((pg_wait_sampling_profileQueries == PGWS_PROFILE_QUERIES_ALL) || \
+	 (pg_wait_sampling_profileQueries == PGWS_PROFILE_QUERIES_TOP && (level) == 0))
 
 /*
  * Calculate max processes count.
@@ -206,30 +214,6 @@ pgws_shmem_size(void)
 	return size;
 }
 
-static bool
-shmem_int_guc_check_hook(int *newval, void **extra, GucSource source)
-{
-	if (UsedShmemSegAddr == NULL)
-		return false;
-	return true;
-}
-
-static bool
-shmem_enum_guc_check_hook(int *newval, void **extra, GucSource source)
-{
-	if (UsedShmemSegAddr == NULL)
-		return false;
-	return true;
-}
-
-static bool
-shmem_bool_guc_check_hook(bool *newval, void **extra, GucSource source)
-{
-	if (UsedShmemSegAddr == NULL)
-		return false;
-	return true;
-}
-
 /*
  * This union allows us to mix the numerous different types of structs
  * that we are organizing.
@@ -244,117 +228,6 @@ typedef union
 	struct config_enum _enum;
 } mixedStruct;
 
-/*
- * Setup new GUCs or modify existsing.
- */
-static void
-setup_gucs()
-{
-	struct config_generic **guc_vars;
-	int			numOpts,
-				i;
-	bool		history_size_found = false,
-				history_period_found = false,
-				profile_period_found = false,
-				profile_pid_found = false,
-				profile_queries_found = false,
-				sample_cpu_found = false;
-
-	get_guc_variables_compat(&guc_vars, &numOpts);
-
-	for (i = 0; i < numOpts; i++)
-	{
-		mixedStruct *var = (mixedStruct *) guc_vars[i];
-		const char *name = var->generic.name;
-
-		if (var->generic.flags & GUC_CUSTOM_PLACEHOLDER)
-			continue;
-
-		if (!strcmp(name, "pg_wait_sampling.history_size"))
-		{
-			history_size_found = true;
-			var->integer.variable = &pgws_collector_hdr->historySize;
-			pgws_collector_hdr->historySize = 5000;
-		}
-		else if (!strcmp(name, "pg_wait_sampling.history_period"))
-		{
-			history_period_found = true;
-			var->integer.variable = &pgws_collector_hdr->historyPeriod;
-			pgws_collector_hdr->historyPeriod = 10;
-		}
-		else if (!strcmp(name, "pg_wait_sampling.profile_period"))
-		{
-			profile_period_found = true;
-			var->integer.variable = &pgws_collector_hdr->profilePeriod;
-			pgws_collector_hdr->profilePeriod = 10;
-		}
-		else if (!strcmp(name, "pg_wait_sampling.profile_pid"))
-		{
-			profile_pid_found = true;
-			var->_bool.variable = &pgws_collector_hdr->profilePid;
-			pgws_collector_hdr->profilePid = true;
-		}
-		else if (!strcmp(name, "pg_wait_sampling.profile_queries"))
-		{
-			profile_queries_found = true;
-			var->_enum.variable = &pgws_collector_hdr->profileQueries;
-			pgws_collector_hdr->profileQueries = PGWS_PROFILE_QUERIES_TOP;
-		}
-		else if (!strcmp(name, "pg_wait_sampling.sample_cpu"))
-		{
-			sample_cpu_found = true;
-			var->_bool.variable = &pgws_collector_hdr->sampleCpu;
-			pgws_collector_hdr->sampleCpu = true;
-		}
-	}
-
-	if (!history_size_found)
-		DefineCustomIntVariable("pg_wait_sampling.history_size",
-				"Sets size of waits history.", NULL,
-				&pgws_collector_hdr->historySize, 5000, 100, INT_MAX,
-				PGC_SUSET, 0, shmem_int_guc_check_hook, NULL, NULL);
-
-	if (!history_period_found)
-		DefineCustomIntVariable("pg_wait_sampling.history_period",
-				"Sets period of waits history sampling.", NULL,
-				&pgws_collector_hdr->historyPeriod, 10, 1, INT_MAX,
-				PGC_SUSET, 0, shmem_int_guc_check_hook, NULL, NULL);
-
-	if (!profile_period_found)
-		DefineCustomIntVariable("pg_wait_sampling.profile_period",
-				"Sets period of waits profile sampling.", NULL,
-				&pgws_collector_hdr->profilePeriod, 10, 1, INT_MAX,
-				PGC_SUSET, 0, shmem_int_guc_check_hook, NULL, NULL);
-
-	if (!profile_pid_found)
-		DefineCustomBoolVariable("pg_wait_sampling.profile_pid",
-				"Sets whether profile should be collected per pid.", NULL,
-				&pgws_collector_hdr->profilePid, true,
-				PGC_SUSET, 0, shmem_bool_guc_check_hook, NULL, NULL);
-
-	if (!profile_queries_found)
-		DefineCustomEnumVariable("pg_wait_sampling.profile_queries",
-				"Sets whether profile should be collected per query.", NULL,
-				&pgws_collector_hdr->profileQueries, PGWS_PROFILE_QUERIES_TOP, pgws_profile_queries_options,
-				PGC_SUSET, 0, shmem_enum_guc_check_hook, NULL, NULL);
-
-	if (!sample_cpu_found)
-		DefineCustomBoolVariable("pg_wait_sampling.sample_cpu",
-		                         "Sets whether not waiting backends should be sampled.", NULL,
-		                         &pgws_collector_hdr->sampleCpu, true,
-		                         PGC_SUSET, 0, shmem_bool_guc_check_hook, NULL, NULL);
-
-	if (history_size_found
-		|| history_period_found
-		|| profile_period_found
-		|| profile_pid_found
-		|| profile_queries_found
-	    || sample_cpu_found)
-	{
-		ProcessConfigFile(PGC_SIGHUP);
-	}
-}
-
 #if PG_VERSION_NUM >= 150000
 /*
  * shmem_request hook: request additional shared memory resources.
@@ -391,17 +264,12 @@ pgws_shmem_startup(void)
 
 		pgws_collector_hdr = shm_toc_allocate(toc, sizeof(CollectorShmqHeader));
 		shm_toc_insert(toc, 0, pgws_collector_hdr);
-		/* needed to please check_GUC_init */
-		pgws_collector_hdr->profileQueries = PGWS_PROFILE_QUERIES_TOP;
 		pgws_collector_mq = shm_toc_allocate(toc, COLLECTOR_QUEUE_SIZE);
 		shm_toc_insert(toc, 1, pgws_collector_mq);
 		pgws_proc_queryids = shm_toc_allocate(toc,
 									sizeof(uint64) * get_max_procs_count());
 		shm_toc_insert(toc, 2, pgws_proc_queryids);
 		MemSet(pgws_proc_queryids, 0, sizeof(uint64) * get_max_procs_count());
-
-		/* Initialize GUC variables in shared memory */
-		setup_gucs();
 	}
 	else
 	{
@@ -482,6 +350,32 @@ _PG_init(void)
 	ExecutorEnd_hook		= pgws_ExecutorEnd;
 	prev_ProcessUtility		= ProcessUtility_hook;
 	ProcessUtility_hook		= pgws_ProcessUtility;
+
+	/* Define GUC variables */
+	DefineCustomIntVariable("pg_wait_sampling.history_size",
+							"Sets size of waits history.", NULL,
+							&pg_wait_sampling_historySize, 5000, 100, INT_MAX,
+							PGC_SIGHUP, 0, NULL, NULL, NULL);
+	DefineCustomIntVariable("pg_wait_sampling.history_period",
+							"Sets period of waits history sampling.", NULL,
+							&pg_wait_sampling_historyPeriod, 10, 1, INT_MAX,
+							PGC_SIGHUP, 0, NULL, NULL, NULL);
+	DefineCustomIntVariable("pg_wait_sampling.profile_period",
+							"Sets period of waits profile sampling.", NULL,
+							&pg_wait_sampling_profilePeriod, 10, 1, INT_MAX,
+							PGC_SIGHUP, 0, NULL, NULL, NULL);
+	DefineCustomBoolVariable("pg_wait_sampling.profile_pid",
+							 "Sets whether profile should be collected per pid.", NULL,
+							 &pg_wait_sampling_profilePid, true,
+							 PGC_SIGHUP, 0, NULL, NULL, NULL);
+	DefineCustomEnumVariable("pg_wait_sampling.profile_queries",
+							 "Sets whether profile should be collected per query.", NULL,
+							 &pg_wait_sampling_profileQueries, PGWS_PROFILE_QUERIES_TOP, pgws_profile_queries_options,
+							 PGC_SIGHUP, 0, NULL, NULL, NULL);
+	DefineCustomBoolVariable("pg_wait_sampling.sample_cpu",
+							 "Sets whether not waiting backends should be sampled.", NULL,
+							 &pg_wait_sampling_sampleCpu, true,
+							 PGC_SIGHUP, 0, NULL, NULL, NULL);
 }
 
 /*
@@ -517,7 +411,7 @@ search_proc(int pid)
 bool
 pgws_should_sample_proc(PGPROC *proc)
 {
-	if (proc->wait_event_info == 0 && !pgws_collector_hdr->sampleCpu)
+	if (proc->wait_event_info == 0 && !pg_wait_sampling_sampleCpu)
 		return false;
 
 	/*
@@ -829,7 +723,7 @@ pg_wait_sampling_get_profile(PG_FUNCTION_ARGS)
 		else
 			nulls[2] = true;
 
-		if (pgws_collector_hdr->profileQueries)
+		if (pg_wait_sampling_profileQueries)
 			values[3] = UInt64GetDatum(item->queryId);
 		else
 			values[3] = (Datum) 0;

pg_wait_sampling.h

@@ -58,14 +58,16 @@ typedef struct
 {
 	Latch		   *latch;
 	SHMRequest		request;
-	int				historySize;
-	int				historyPeriod;
-	int				profilePeriod;
-	bool			profilePid;
-	int				profileQueries;
-	bool			sampleCpu;
 } CollectorShmqHeader;
 
+/* GUC variables */
+extern int pg_wait_sampling_historySize;
+extern int pg_wait_sampling_historyPeriod;
+extern int pg_wait_sampling_profilePeriod;
+extern bool pg_wait_sampling_profilePid;
+extern int pg_wait_sampling_profileQueries;
+extern bool pg_wait_sampling_sampleCpu;
+
 /* pg_wait_sampling.c */
 extern CollectorShmqHeader *pgws_collector_hdr;
 extern shm_mq			   *pgws_collector_mq;