0001-Provide-useful-values-for-postgres-C-with-runtime-co.patch

From aba09219bbcbdb5d9747c63dbf75380c774873e7 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <bossartn@amazon.com>
Date: Thu, 9 Sep 2021 21:42:41 +0000
Subject: [PATCH 1/1] Provide useful values for 'postgres -C' with
 runtime-computed GUCs.

The -C option is handled before a small subset of GUCs that are
computed at runtime are initialized.  Unfortunately, we cannot move
this handling to after they are initialized without disallowing
'postgres -C' on a running server.  One notable reason for this is
that loadable libraries' _PG_init() functions are called before all
runtime-computed GUCs are initialized, and this is not guaranteed
to be safe to do on running servers.

In order to provide useful values for 'postgres -C' for runtime-
computed GUCs, this change adds a new section for handling just
these GUCs just before shared memory is initialized.  While users
won't be able to use -C for runtime-computed GUCs on running
servers, providing a useful value with this restriction seems
better than not providing a useful value at all.
---
 doc/src/sgml/ref/postgres-ref.sgml  | 12 ++++++--
 doc/src/sgml/runtime.sgml           | 33 +++++++-------------
 src/backend/bootstrap/bootstrap.c   |  2 +-
 src/backend/postmaster/postmaster.c | 52 +++++++++++++++++++++++++++-----
 src/backend/tcop/postgres.c         |  2 +-
 src/backend/utils/init/miscinit.c   | 60 ++++++++++++++++++++++++++++---------
 src/backend/utils/misc/guc.c        |  8 ++---
 src/include/miscadmin.h             |  3 +-
 src/include/utils/guc.h             |  6 ++++
 9 files changed, 125 insertions(+), 53 deletions(-)

diff --git a/doc/src/sgml/ref/postgres-ref.sgml b/doc/src/sgml/ref/postgres-ref.sgml
index 4aaa7abe1a..89cc3cdb4e 100644
--- a/doc/src/sgml/ref/postgres-ref.sgml
+++ b/doc/src/sgml/ref/postgres-ref.sgml
@@ -133,13 +133,21 @@ PostgreSQL documentation
       <listitem>
        <para>
         Prints the value of the named run-time parameter, and exits.
-        (See the <option>-c</option> option above for details.)  This can
-        be used on a running server, and returns values from
+        (See the <option>-c</option> option above for details.)  This
+        returns values from
         <filename>postgresql.conf</filename>, modified by any parameters
         supplied in this invocation.  It does not reflect parameters
         supplied when the cluster was started.
        </para>
 
+       <para>
+        This can be used on a running server for most parameters.  However,
+        the server must be shut down for some runtime-computed parameters
+        (e.g., <xref linkend="guc-huge-pages-required"/>,
+        <xref linkend="guc-shared-memory-size"/>, and
+        <xref linkend="guc-wal-segment-size"/>).
+       </para>
+
        <para>
         This option is meant for other programs that interact with a server
         instance, such as <xref linkend="app-pg-ctl"/>, to query configuration
diff --git a/doc/src/sgml/runtime.sgml b/doc/src/sgml/runtime.sgml
index f1cbc1d9e9..d955639900 100644
--- a/doc/src/sgml/runtime.sgml
+++ b/doc/src/sgml/runtime.sgml
@@ -1442,32 +1442,21 @@ export PG_OOM_ADJUST_VALUE=0
     with <varname>CONFIG_HUGETLBFS=y</varname> and
     <varname>CONFIG_HUGETLB_PAGE=y</varname>. You will also have to configure
     the operating system to provide enough huge pages of the desired size.
-    To estimate the number of huge pages needed, start
-    <productname>PostgreSQL</productname> without huge pages enabled and check
-    the postmaster's anonymous shared memory segment size, as well as the
-    system's default and supported huge page sizes, using the
-    <filename>/proc</filename> and <filename>/sys</filename> file systems.
-    This might look like:
+    To estimate the number of huge pages needed, use the
+    <command>postgres</command> command to see the value of
+    <xref linkend="guc-huge-pages-required"/>.  This might look like:
 <programlisting>
-$ <userinput>head -1 $PGDATA/postmaster.pid</userinput>
-4170
-$ <userinput>pmap 4170 | awk '/rw-s/ &amp;&amp; /zero/ {print $2}'</userinput>
-6490428K
-$ <userinput>grep ^Hugepagesize /proc/meminfo</userinput>
-Hugepagesize:       2048 kB
-$ <userinput>ls /sys/kernel/mm/hugepages</userinput>
-hugepages-1048576kB  hugepages-2048kB
+$ <userinput>postgres -D $PGDATA -C huge_pages_required</userinput>
+3170
 </programlisting>
 
-     In this example the default is 2MB, but you can also explicitly request
-     either 2MB or 1GB with <xref linkend="guc-huge-page-size"/>.
+     Note that you can explicitly request either 2MB or 1GB huge pages with
+     <xref linkend="guc-huge-page-size"/>.
 
-     Assuming <literal>2MB</literal> huge pages,
-     <literal>6490428</literal> / <literal>2048</literal> gives approximately
-     <literal>3169.154</literal>, so in this example we need at
-     least <literal>3170</literal> huge pages.  A larger setting would be
-     appropriate if other programs on the machine also need huge pages.
-     We can set this with:
+     While we need at least <literal>3170</literal> huge pages in this
+     example, a larger setting would be appropriate if other programs on
+     the machine also need huge pages.  We can allocate the huge pages
+     with:
 <programlisting>
 # <userinput>sysctl -w vm.nr_hugepages=3170</userinput>
 </programlisting>
diff --git a/src/backend/bootstrap/bootstrap.c b/src/backend/bootstrap/bootstrap.c
index 48615c0ebc..2c9e832951 100644
--- a/src/backend/bootstrap/bootstrap.c
+++ b/src/backend/bootstrap/bootstrap.c
@@ -317,7 +317,7 @@ BootstrapModeMain(int argc, char *argv[], bool check_only)
 	checkDataDir();
 	ChangeToDataDir();
 
-	CreateDataDirLockFile(false);
+	CreateDataDirLockFile(false, NULL);
 
 	SetProcessingMode(BootstrapProcessing);
 	IgnoreSystemIndexes = true;
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index b2fe420c3c..58136aac09 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -896,15 +896,31 @@ PostmasterMain(int argc, char *argv[])
 	if (output_config_variable != NULL)
 	{
 		/*
-		 * "-C guc" was specified, so print GUC's value and exit.  No extra
-		 * permission check is needed because the user is reading inside the
-		 * data dir.
+		 * If this is a runtime-computed GUC, it hasn't yet been initialized,
+		 * and the present value is not useful.  However, this is a convenient
+		 * place to print the value for most GUCs because it is safe to run
+		 * postmaster startup to this point even if the server is already
+		 * running.  For the handful of runtime-computed GUCs that we can't
+		 * provide meaningful values for yet, we wait until later in postmaster
+		 * startup to print the value.  We won't be able to use -C on running
+		 * servers for those GUCs, but otherwise this option is unusable for
+		 * them.
 		 */
-		const char *config_val = GetConfigOption(output_config_variable,
-												 false, false);
+		int flags = GetConfigOptionFlags(output_config_variable, true);
 
-		puts(config_val ? config_val : "");
-		ExitPostmaster(0);
+		if ((flags & GUC_RUNTIME_COMPUTED) == 0)
+		{
+			/*
+			 * "-C guc" was specified, so print GUC's value and exit.  No extra
+			 * permission check is needed because the user is reading inside
+			 * the data dir.
+			 */
+			const char *config_val = GetConfigOption(output_config_variable,
+													 false, false);
+
+			puts(config_val ? config_val : "");
+			ExitPostmaster(0);
+		}
 	}
 
 	/* Verify that DataDir looks reasonable */
@@ -983,7 +999,7 @@ PostmasterMain(int argc, char *argv[])
 	 * so it must happen before opening sockets so that at exit, the socket
 	 * lockfiles go away after CloseServerPorts runs.
 	 */
-	CreateDataDirLockFile(true);
+	CreateDataDirLockFile(true, output_config_variable);
 
 	/*
 	 * Read the control file (for error checking and config info).
@@ -1033,6 +1049,26 @@ PostmasterMain(int argc, char *argv[])
 	 */
 	InitializeShmemGUCs();
 
+	/*
+	 * If -C was specified with a runtime-computed GUC, we held off printing
+	 * the value earlier, as the GUC was not yet initialized.  We handle -C for
+	 * most GUCs before we lock the data directory so that the option may be
+	 * used on a running server.  However, a handful of GUCs are runtime-
+	 * computed and do not have meaningful values until after locking the data
+	 * directory, and we cannot safely calculate their values earlier on a
+	 * running server.  At this point, such GUCs should be properly
+	 * initialized, and we haven't yet set up shared memory, so this is a good
+	 * time to handle the -C option for these special GUCs.
+	 */
+	if (output_config_variable != NULL)
+	{
+		const char *config_val = GetConfigOption(output_config_variable,
+												 false, false);
+
+		puts(config_val ? config_val : "");
+		ExitPostmaster(0);
+	}
+
 	/*
 	 * Set up shared memory and semaphores.
 	 */
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 58b5960e27..d84759ae59 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -4043,7 +4043,7 @@ PostgresMain(int argc, char *argv[],
 		/*
 		 * Create lockfile for data directory.
 		 */
-		CreateDataDirLockFile(false);
+		CreateDataDirLockFile(false, NULL);
 
 		/* read control file (error checking and contains config ) */
 		LocalProcessControlFile(false);
diff --git a/src/backend/utils/init/miscinit.c b/src/backend/utils/init/miscinit.c
index 88801374b5..a63aba6f57 100644
--- a/src/backend/utils/init/miscinit.c
+++ b/src/backend/utils/init/miscinit.c
@@ -952,6 +952,7 @@ static void
 UnlinkLockFiles(int status, Datum arg)
 {
 	ListCell   *l;
+	bool		logShutDown = DatumGetBool(arg);
 
 	foreach(l, lock_files)
 	{
@@ -968,10 +969,10 @@ UnlinkLockFiles(int status, Datum arg)
 	 * of a postmaster or standalone backend, while we won't come here at all
 	 * when exiting postmaster child processes.  Therefore, this is a good
 	 * place to log completion of shutdown.  We could alternatively teach
-	 * proc_exit() to do it, but that seems uglier.  In a standalone backend,
-	 * use NOTICE elevel to be less chatty.
+	 * proc_exit() to do it, but that seems uglier.  In a standalone backend
+	 * and when logShutDown is false, use NOTICE elevel to be less chatty.
 	 */
-	ereport(IsPostmasterEnvironment ? LOG : NOTICE,
+	ereport(IsPostmasterEnvironment && logShutDown ? LOG : NOTICE,
 			(errmsg("database system is shut down")));
 }
 
@@ -986,7 +987,8 @@ UnlinkLockFiles(int status, Datum arg)
 static void
 CreateLockFile(const char *filename, bool amPostmaster,
 			   const char *socketDir,
-			   bool isDDLock, const char *refName)
+			   bool isDDLock, const char *refName,
+			   const char *output_config_variable)
 {
 	int			fd;
 	char		buffer[MAXPGPATH * 2 + 256];
@@ -999,6 +1001,18 @@ CreateLockFile(const char *filename, bool amPostmaster,
 				my_gp_pid;
 	const char *envvar;
 
+	/*
+	 * If output_config_variable is not NULL, we try to add some useful context
+	 * to the FATAL messages to make it clear why 'postgres -C' is failing for
+	 * this particular GUC (only a small set of runtime-computed GUCs require
+	 * the server to be shut down).
+	 */
+#define LOCK_FILE_ERRDETAIL (output_config_variable ? \
+							 errdetail("Runtime-computed GUC \"%s\" cannot " \
+									   "be viewed on a running server.", \
+									   output_config_variable) : \
+							 0)
+
 	/*
 	 * If the PID in the lockfile is our own PID or our parent's or
 	 * grandparent's PID, then the file must be stale (probably left over from
@@ -1060,7 +1074,8 @@ CreateLockFile(const char *filename, bool amPostmaster,
 			ereport(FATAL,
 					(errcode_for_file_access(),
 					 errmsg("could not create lock file \"%s\": %m",
-							filename)));
+							filename),
+					 LOCK_FILE_ERRDETAIL));
 
 		/*
 		 * Read the file to get the old owner's PID.  Note race condition
@@ -1074,14 +1089,16 @@ CreateLockFile(const char *filename, bool amPostmaster,
 			ereport(FATAL,
 					(errcode_for_file_access(),
 					 errmsg("could not open lock file \"%s\": %m",
-							filename)));
+							filename),
+					 LOCK_FILE_ERRDETAIL));
 		}
 		pgstat_report_wait_start(WAIT_EVENT_LOCK_FILE_CREATE_READ);
 		if ((len = read(fd, buffer, sizeof(buffer) - 1)) < 0)
 			ereport(FATAL,
 					(errcode_for_file_access(),
 					 errmsg("could not read lock file \"%s\": %m",
-							filename)));
+							filename),
+					 LOCK_FILE_ERRDETAIL));
 		pgstat_report_wait_end();
 		close(fd);
 
@@ -1090,6 +1107,7 @@ CreateLockFile(const char *filename, bool amPostmaster,
 			ereport(FATAL,
 					(errcode(ERRCODE_LOCK_FILE_EXISTS),
 					 errmsg("lock file \"%s\" is empty", filename),
+					 LOCK_FILE_ERRDETAIL,
 					 errhint("Either another server is starting, or the lock file is the remnant of a previous server startup crash.")));
 		}
 
@@ -1136,6 +1154,7 @@ CreateLockFile(const char *filename, bool amPostmaster,
 						(errcode(ERRCODE_LOCK_FILE_EXISTS),
 						 errmsg("lock file \"%s\" already exists",
 								filename),
+						 LOCK_FILE_ERRDETAIL,
 						 isDDLock ?
 						 (encoded_pid < 0 ?
 						  errhint("Is another postgres (PID %d) running in data directory \"%s\"?",
@@ -1183,6 +1202,7 @@ CreateLockFile(const char *filename, bool amPostmaster,
 							(errcode(ERRCODE_LOCK_FILE_EXISTS),
 							 errmsg("pre-existing shared memory block (key %lu, ID %lu) is still in use",
 									id1, id2),
+							 LOCK_FILE_ERRDETAIL,
 							 errhint("Terminate any old server processes associated with data directory \"%s\".",
 									 refName)));
 			}
@@ -1198,6 +1218,7 @@ CreateLockFile(const char *filename, bool amPostmaster,
 					(errcode_for_file_access(),
 					 errmsg("could not remove old lock file \"%s\": %m",
 							filename),
+					 LOCK_FILE_ERRDETAIL,
 					 errhint("The file seems accidentally left over, but "
 							 "it could not be removed. Please remove the file "
 							 "by hand and try again.")));
@@ -1235,7 +1256,8 @@ CreateLockFile(const char *filename, bool amPostmaster,
 		errno = save_errno ? save_errno : ENOSPC;
 		ereport(FATAL,
 				(errcode_for_file_access(),
-				 errmsg("could not write lock file \"%s\": %m", filename)));
+				 errmsg("could not write lock file \"%s\": %m", filename),
+				 LOCK_FILE_ERRDETAIL));
 	}
 	pgstat_report_wait_end();
 
@@ -1249,7 +1271,8 @@ CreateLockFile(const char *filename, bool amPostmaster,
 		errno = save_errno;
 		ereport(FATAL,
 				(errcode_for_file_access(),
-				 errmsg("could not write lock file \"%s\": %m", filename)));
+				 errmsg("could not write lock file \"%s\": %m", filename),
+				 LOCK_FILE_ERRDETAIL));
 	}
 	pgstat_report_wait_end();
 	if (close(fd) != 0)
@@ -1260,16 +1283,21 @@ CreateLockFile(const char *filename, bool amPostmaster,
 		errno = save_errno;
 		ereport(FATAL,
 				(errcode_for_file_access(),
-				 errmsg("could not write lock file \"%s\": %m", filename)));
+				 errmsg("could not write lock file \"%s\": %m", filename),
+				 LOCK_FILE_ERRDETAIL));
 	}
 
 	/*
 	 * Arrange to unlink the lock file(s) at proc_exit.  If this is the first
 	 * one, set up the on_proc_exit function to do it; then add this lock file
 	 * to the list of files to unlink.
+	 *
+	 * If output_config_variable is not NULL, we ask UnlinkLockFiles to be less
+	 * chatty to improve the user experience with 'postgres -C'.
 	 */
 	if (lock_files == NIL)
-		on_proc_exit(UnlinkLockFiles, 0);
+		on_proc_exit(UnlinkLockFiles,
+					 BoolGetDatum(output_config_variable == NULL));
 
 	/*
 	 * Use lcons so that the lock files are unlinked in reverse order of
@@ -1287,11 +1315,15 @@ CreateLockFile(const char *filename, bool amPostmaster,
  *
  * Note that the socket directory path line is initially written as empty.
  * postmaster.c will rewrite it upon creating the first Unix socket.
+ *
+ * If output_config_variable is not NULL (i.e., we are running 'postgres
+ * -C'), the logs we emit will be slightly adjusted to provide a better
+ * user experience.
  */
 void
-CreateDataDirLockFile(bool amPostmaster)
+CreateDataDirLockFile(bool amPostmaster, const char *output_config_variable)
 {
-	CreateLockFile(DIRECTORY_LOCK_FILE, amPostmaster, "", true, DataDir);
+	CreateLockFile(DIRECTORY_LOCK_FILE, amPostmaster, "", true, DataDir, output_config_variable);
 }
 
 /*
@@ -1304,7 +1336,7 @@ CreateSocketLockFile(const char *socketfile, bool amPostmaster,
 	char		lockfile[MAXPGPATH];
 
 	snprintf(lockfile, sizeof(lockfile), "%s.lock", socketfile);
-	CreateLockFile(lockfile, amPostmaster, socketDir, false, socketfile);
+	CreateLockFile(lockfile, amPostmaster, socketDir, false, socketfile, NULL);
 }
 
 /*
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 23236fa4c3..a6e4fcc24e 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -1983,7 +1983,7 @@ static struct config_bool ConfigureNamesBool[] =
 		{"data_checksums", PGC_INTERNAL, PRESET_OPTIONS,
 			gettext_noop("Shows whether data checksums are turned on for this cluster."),
 			NULL,
-			GUC_NOT_IN_SAMPLE | GUC_DISALLOW_IN_FILE
+			GUC_NOT_IN_SAMPLE | GUC_DISALLOW_IN_FILE | GUC_RUNTIME_COMPUTED
 		},
 		&data_checksums,
 		false,
@@ -2342,7 +2342,7 @@ static struct config_int ConfigureNamesInt[] =
 		{"shared_memory_size", PGC_INTERNAL, PRESET_OPTIONS,
 			gettext_noop("Shows the size of the server's main shared memory area (rounded up to the nearest MB)."),
 			NULL,
-			GUC_NOT_IN_SAMPLE | GUC_DISALLOW_IN_FILE | GUC_UNIT_MB
+			GUC_NOT_IN_SAMPLE | GUC_DISALLOW_IN_FILE | GUC_UNIT_MB | GUC_RUNTIME_COMPUTED
 		},
 		&shared_memory_size_mb,
 		0, 0, INT_MAX,
@@ -2407,7 +2407,7 @@ static struct config_int ConfigureNamesInt[] =
 						 "in the form accepted by the chmod and umask system "
 						 "calls. (To use the customary octal format the number "
 						 "must start with a 0 (zero).)"),
-			GUC_NOT_IN_SAMPLE | GUC_DISALLOW_IN_FILE
+			GUC_NOT_IN_SAMPLE | GUC_DISALLOW_IN_FILE | GUC_RUNTIME_COMPUTED
 		},
 		&data_directory_mode,
 		0700, 0000, 0777,
@@ -3231,7 +3231,7 @@ static struct config_int ConfigureNamesInt[] =
 		{"wal_segment_size", PGC_INTERNAL, PRESET_OPTIONS,
 			gettext_noop("Shows the size of write ahead log segments."),
 			NULL,
-			GUC_UNIT_BYTE | GUC_NOT_IN_SAMPLE | GUC_DISALLOW_IN_FILE
+			GUC_UNIT_BYTE | GUC_NOT_IN_SAMPLE | GUC_DISALLOW_IN_FILE | GUC_RUNTIME_COMPUTED
 		},
 		&wal_segment_size,
 		DEFAULT_XLOG_SEG_SIZE,
diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h
index 2e2e9a364a..d630ffa310 100644
--- a/src/include/miscadmin.h
+++ b/src/include/miscadmin.h
@@ -466,7 +466,8 @@ extern char *session_preload_libraries_string;
 extern char *shared_preload_libraries_string;
 extern char *local_preload_libraries_string;
 
-extern void CreateDataDirLockFile(bool amPostmaster);
+extern void CreateDataDirLockFile(bool amPostmaster,
+								  const char *output_config_variable);
 extern void CreateSocketLockFile(const char *socketfile, bool amPostmaster,
 								 const char *socketDir);
 extern void TouchSocketLockFiles(void);
diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h
index a7c3a4958e..aa18d304ac 100644
--- a/src/include/utils/guc.h
+++ b/src/include/utils/guc.h
@@ -229,6 +229,12 @@ typedef enum
 
 #define GUC_EXPLAIN			  0x100000	/* include in explain */
 
+/*
+ * GUC_RUNTIME_COMPUTED is intended for runtime-computed GUCs that are only
+ * available via 'postgres -C' if the server is not running.
+ */
+#define GUC_RUNTIME_COMPUTED  0x200000
+
 #define GUC_UNIT				(GUC_UNIT_MEMORY | GUC_UNIT_TIME)
 
 
-- 
2.16.6