On Tue, Sep 14, 2021 at 06:00:44PM +0000, Bossart, Nathan wrote: > I think I see more support for shared_memory_size_in_huge_pages than > for huge_pages_needed_for_shared_memory at the moment. I'll update > the patch set in the next day or two to use > shared_memory_size_in_huge_pages unless something changes in the > meantime.
I have been looking at the patch to add the new GUC flag and the new sequence for postgres -C, and we could have some TAP tests. There were two places that made sense to me: pg_checksums/t/002_actions.pl and recovery/t/017_shm.pl. I have chosen the former as it has coverage across more platforms, and used data_checksums for this purpose, with an extra positive test to check for the case where a GUC can be queried while the server is running. There are four parameters that are updated here: - shared_memory_size - wal_segment_size - data_checksums - data_directory_mode That looks sensible, after looking at the full set of GUCs. Attached is a refreshed patch (commit message is the same as v9 for now), with some minor tweaks and the tests. Thoughts? -- Michael
From e24d151efe45e1bc7048ec72ea66097c94f633be Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@paquier.xyz>
Date: Wed, 15 Sep 2021 12:02:32 +0900
Subject: [PATCH v10] 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.
---
src/include/utils/guc.h | 6 ++++
src/backend/postmaster/postmaster.c | 50 +++++++++++++++++++++++----
src/backend/utils/misc/guc.c | 8 ++---
src/bin/pg_checksums/t/002_actions.pl | 21 ++++++++++-
doc/src/sgml/ref/postgres-ref.sgml | 11 ++++--
5 files changed, 82 insertions(+), 14 deletions(-)
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)
diff --git a/src/backend/postmaster/postmaster.c
b/src/backend/postmaster/postmaster.c
index b2fe420c3c..f5b91c7af7 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 */
@@ -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/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/bin/pg_checksums/t/002_actions.pl
b/src/bin/pg_checksums/t/002_actions.pl
index a18c104a94..e98586c3f8 100644
--- a/src/bin/pg_checksums/t/002_actions.pl
+++ b/src/bin/pg_checksums/t/002_actions.pl
@@ -10,7 +10,7 @@ use PostgresNode;
use TestLib;
use Fcntl qw(:seek);
-use Test::More tests => 63;
+use Test::More tests => 69;
# Utility routine to create and check a table with corrupted checksums
@@ -178,11 +178,30 @@ command_fails(
[ 'pg_checksums', '--enable', '--filenode', '1234', '-D', $pgdata ],
"fails when relfilenodes are requested and action is --enable");
+# Test postgres -C for an offline cluster.
+# Run-time GUCs are safe to query here. Note that a lock file is created,
+# then unlinked, leading to an extra LOG entry showing in stderr.
+command_checks_all(
+ [ 'postgres', '-D', $pgdata, '-C', 'data_checksums' ],
+ 0,
+ [qr/^on$/],
+ # LOG entry when unlinking lock file.
+ [qr/database system is shut down/],
+ 'data_checksums=on is reported on an offline cluster');
+
# Checks cannot happen with an online cluster
$node->start;
command_fails([ 'pg_checksums', '--check', '-D', $pgdata ],
"fails with online cluster");
+# Test postgres -C on an online cluster.
+command_fails_like(
+ [ 'postgres', '-D', $pgdata, '-C', 'data_checksums' ],
+ qr/lock file .* already exists/,
+ 'data_checksums is not reported on an online cluster');
+command_ok([ 'postgres', '-D', $pgdata, '-C', 'work_mem' ],
+ 'non-runtime parameter is reported on an online cluster');
+
# Check corruption of table on default tablespace.
check_relation_corruption($node, 'corrupt1', 'pg_default');
diff --git a/doc/src/sgml/ref/postgres-ref.sgml
b/doc/src/sgml/ref/postgres-ref.sgml
index 4aaa7abe1a..02142ffab2 100644
--- a/doc/src/sgml/ref/postgres-ref.sgml
+++ b/doc/src/sgml/ref/postgres-ref.sgml
@@ -133,13 +133,20 @@ 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-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
--
2.33.0
signature.asc
Description: PGP signature
