From aba09219bbcbdb5d9747c63dbf75380c774873e7 Mon Sep 17 00:00:00 2001 From: Nathan Bossart 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 Prints the value of the named run-time parameter, and exits. - (See the option above for details.) This can - be used on a running server, and returns values from + (See the option above for details.) This + returns values from postgresql.conf, modified by any parameters supplied in this invocation. It does not reflect parameters supplied when the cluster was started. + + 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., , + , and + ). + + This option is meant for other programs that interact with a server instance, such as , 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 CONFIG_HUGETLBFS=y and CONFIG_HUGETLB_PAGE=y. 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 - PostgreSQL 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 - /proc and /sys file systems. - This might look like: + To estimate the number of huge pages needed, use the + postgres command to see the value of + . This might look like: -$ head -1 $PGDATA/postmaster.pid -4170 -$ pmap 4170 | awk '/rw-s/ && /zero/ {print $2}' -6490428K -$ grep ^Hugepagesize /proc/meminfo -Hugepagesize: 2048 kB -$ ls /sys/kernel/mm/hugepages -hugepages-1048576kB hugepages-2048kB +$ postgres -D $PGDATA -C huge_pages_required +3170 - In this example the default is 2MB, but you can also explicitly request - either 2MB or 1GB with . + Note that you can explicitly request either 2MB or 1GB huge pages with + . - Assuming 2MB huge pages, - 6490428 / 2048 gives approximately - 3169.154, so in this example we need at - least 3170 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 3170 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: # sysctl -w vm.nr_hugepages=3170 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