On Tue, Jul 19, 2022 at 03:04:27PM +0900, Kyotaro Horiguchi wrote:
> At Wed, 13 Jul 2022 18:54:45 -0500, Justin Pryzby <pry...@telsasoft.com>
> wrote in
> > On Thu, Jul 14, 2022 at 08:46:02AM +0900, Michael Paquier wrote:
> > > On Wed, Jul 13, 2022 at 12:30:00PM -0500, Justin Pryzby wrote:
> > > > How did you make this list ? Was it by excluding things that failed
> > > > for you ?
>
> Yes. I didn't confirm each variable. They are the variables differ on
> RHEL-family OSes. io_concurrency differs according to
> USE_PREFETCH. Regarding to effects of macro definitions, I searched
> guc.c for non-GUC_NOT_IN_SAMPLE variables with macro-affected defaults.
I think you'd also need to handle the ones which are changed by initdb.c.
This patch takes Andres' suggestion.
The list of GUCs I flagged is probably incomplete, maybe inaccurate, and at
least up for discussion.
BTW I still think it might have been better to leave pg_settings_get_flags()
deliberately undocumented.
--
Justin
>From 5f9a56a9c0d73a221e55b2c02e006d3104f4937b Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryz...@telsasoft.com>
Date: Tue, 19 Jul 2022 08:31:56 -0500
Subject: [PATCH 1/2] wip: add DYNAMIC_DEFAULT for settings which are vary by
configure flags or platform or initdb
---
doc/src/sgml/func.sgml | 4 ++++
src/backend/utils/misc/guc.c | 36 +++++++++++++++++++++++-------------
src/include/utils/guc.h | 6 ++++--
3 files changed, 31 insertions(+), 15 deletions(-)
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index e3e24e185a4..0fd6234eaf2 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -24977,6 +24977,10 @@ SELECT currval(pg_get_serial_sequence('sometable', 'id'));
an empty array if the GUC exists but there are no flags to show.
Only the most useful flags are exposed, as of the following:
<simplelist>
+ <member>
+ <literal>DYNAMIC_DEFAULT</literal>: parameters whose default varies by
+ platform, or compile-time options, or initdb.
+ </member>
<member>
<literal>EXPLAIN</literal>: parameters included in
<command>EXPLAIN (SETTINGS)</command> commands.
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 0328029d430..dbbadc5a475 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -1435,7 +1435,7 @@ static struct config_bool ConfigureNamesBool[] =
{"debug_assertions", PGC_INTERNAL, PRESET_OPTIONS,
gettext_noop("Shows whether the running server has assertion checks enabled."),
NULL,
- GUC_NOT_IN_SAMPLE | GUC_DISALLOW_IN_FILE
+ GUC_NOT_IN_SAMPLE | GUC_DISALLOW_IN_FILE | GUC_DYNAMIC_DEFAULT
},
&assert_enabled,
#ifdef USE_ASSERT_CHECKING
@@ -1611,7 +1611,8 @@ static struct config_bool ConfigureNamesBool[] =
{
{"update_process_title", PGC_SUSET, PROCESS_TITLE,
gettext_noop("Updates the process title to show the active SQL command."),
- gettext_noop("Enables updating of the process title every time a new SQL command is received by the server.")
+ gettext_noop("Enables updating of the process title every time a new SQL command is received by the server."),
+ GUC_DYNAMIC_DEFAULT
},
&update_process_title,
#ifdef WIN32
@@ -2362,6 +2363,7 @@ static struct config_int ConfigureNamesInt[] =
gettext_noop("Sets the maximum number of concurrent connections."),
NULL
},
+
&MaxConnections,
100, 1, MAX_BACKENDS,
check_maxconnections, NULL, NULL
@@ -2397,7 +2399,7 @@ static struct config_int ConfigureNamesInt[] =
{"shared_buffers", PGC_POSTMASTER, RESOURCES_MEM,
gettext_noop("Sets the number of shared memory buffers used by the server."),
NULL,
- GUC_UNIT_BLOCKS
+ GUC_UNIT_BLOCKS | GUC_DYNAMIC_DEFAULT
},
&NBuffers,
16384, 16, INT_MAX / 2,
@@ -3142,7 +3144,7 @@ static struct config_int ConfigureNamesInt[] =
RESOURCES_ASYNCHRONOUS,
gettext_noop("Number of simultaneous requests that can be handled efficiently by the disk subsystem."),
NULL,
- GUC_EXPLAIN
+ GUC_EXPLAIN|GUC_DYNAMIC_DEFAULT
},
&effective_io_concurrency,
#ifdef USE_PREFETCH
@@ -3160,7 +3162,7 @@ static struct config_int ConfigureNamesInt[] =
RESOURCES_ASYNCHRONOUS,
gettext_noop("A variant of effective_io_concurrency that is used for maintenance work."),
NULL,
- GUC_EXPLAIN
+ GUC_EXPLAIN|GUC_DYNAMIC_DEFAULT
},
&maintenance_io_concurrency,
#ifdef USE_PREFETCH
@@ -3327,9 +3329,11 @@ static struct config_int ConfigureNamesInt[] =
gettext_noop("Shows the size of write ahead log segments."),
NULL,
GUC_UNIT_BYTE | GUC_NOT_IN_SAMPLE | GUC_DISALLOW_IN_FILE | GUC_RUNTIME_COMPUTED
+
},
&wal_segment_size,
DEFAULT_XLOG_SEG_SIZE,
+
WalSegMinSize,
WalSegMaxSize,
NULL, NULL, NULL
@@ -3523,6 +3527,7 @@ static struct config_int ConfigureNamesInt[] =
GUC_UNIT_BLOCKS | GUC_EXPLAIN,
},
&effective_cache_size,
+
DEFAULT_EFFECTIVE_CACHE_SIZE, 1, INT_MAX,
NULL, NULL, NULL
},
@@ -3620,7 +3625,7 @@ static struct config_int ConfigureNamesInt[] =
{"debug_discard_caches", PGC_SUSET, DEVELOPER_OPTIONS,
gettext_noop("Aggressively flush system caches for debugging purposes."),
NULL,
- GUC_NOT_IN_SAMPLE
+ GUC_NOT_IN_SAMPLE | GUC_DYNAMIC_DEFAULT
},
&debug_discard_caches,
#ifdef DISCARD_CACHES_ENABLED
@@ -4447,7 +4452,7 @@ static struct config_string ConfigureNamesString[] =
{"unix_socket_directories", PGC_POSTMASTER, CONN_AUTH_SETTINGS,
gettext_noop("Sets the directories where Unix-domain sockets will be created."),
NULL,
- GUC_LIST_INPUT | GUC_LIST_QUOTE | GUC_SUPERUSER_ONLY
+ GUC_LIST_INPUT | GUC_LIST_QUOTE | GUC_SUPERUSER_ONLY | GUC_DYNAMIC_DEFAULT
},
&Unix_socket_directories,
#ifdef HAVE_UNIX_SOCKETS
@@ -4532,7 +4537,7 @@ static struct config_string ConfigureNamesString[] =
{"ssl_library", PGC_INTERNAL, PRESET_OPTIONS,
gettext_noop("Shows the name of the SSL library."),
NULL,
- GUC_NOT_IN_SAMPLE | GUC_DISALLOW_IN_FILE
+ GUC_NOT_IN_SAMPLE | GUC_DISALLOW_IN_FILE | GUC_DYNAMIC_DEFAULT
},
&ssl_library,
#ifdef USE_SSL
@@ -4618,7 +4623,7 @@ static struct config_string ConfigureNamesString[] =
{"ssl_ciphers", PGC_SIGHUP, CONN_AUTH_SSL,
gettext_noop("Sets the list of allowed SSL ciphers."),
NULL,
- GUC_SUPERUSER_ONLY
+ GUC_SUPERUSER_ONLY | GUC_DYNAMIC_DEFAULT
},
&SSLCipherSuites,
#ifdef USE_OPENSSL
@@ -4633,7 +4638,7 @@ static struct config_string ConfigureNamesString[] =
{"ssl_ecdh_curve", PGC_SIGHUP, CONN_AUTH_SSL,
gettext_noop("Sets the curve to use for ECDH."),
NULL,
- GUC_SUPERUSER_ONLY
+ GUC_SUPERUSER_ONLY | GUC_DYNAMIC_DEFAULT
},
&SSLECDHCurve,
#ifdef USE_SSL
@@ -4871,7 +4876,8 @@ static struct config_enum ConfigureNamesEnum[] =
{
{"syslog_facility", PGC_SIGHUP, LOGGING_WHERE,
gettext_noop("Sets the syslog \"facility\" to be used when syslog enabled."),
- NULL
+ NULL,
+ GUC_DYNAMIC_DEFAULT
},
&syslog_facility,
#ifdef HAVE_SYSLOG
@@ -4984,7 +4990,8 @@ static struct config_enum ConfigureNamesEnum[] =
{
{"dynamic_shared_memory_type", PGC_POSTMASTER, RESOURCES_MEM,
gettext_noop("Selects the dynamic shared memory implementation used."),
- NULL
+ NULL,
+ GUC_DYNAMIC_DEFAULT
},
&dynamic_shared_memory_type,
DEFAULT_DYNAMIC_SHARED_MEMORY_TYPE, dynamic_shared_memory_options,
@@ -5008,6 +5015,7 @@ static struct config_enum ConfigureNamesEnum[] =
},
&sync_method,
DEFAULT_SYNC_METHOD, sync_method_options,
+
NULL, assign_xlog_sync_method, NULL
},
@@ -9943,7 +9951,7 @@ GetConfigOptionByName(const char *name, const char **varname, bool missing_ok)
Datum
pg_settings_get_flags(PG_FUNCTION_ARGS)
{
-#define MAX_GUC_FLAGS 5
+#define MAX_GUC_FLAGS 6
char *varname = TextDatumGetCString(PG_GETARG_DATUM(0));
struct config_generic *record;
int cnt = 0;
@@ -9956,6 +9964,8 @@ pg_settings_get_flags(PG_FUNCTION_ARGS)
if (record == NULL)
PG_RETURN_NULL();
+ if (record->flags & GUC_DYNAMIC_DEFAULT)
+ flags[cnt++] = CStringGetTextDatum("DYNAMIC_DEFAULT");
if (record->flags & GUC_EXPLAIN)
flags[cnt++] = CStringGetTextDatum("EXPLAIN");
if (record->flags & GUC_NO_RESET_ALL)
diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h
index 4d0920c42e2..bb09c507f81 100644
--- a/src/include/utils/guc.h
+++ b/src/include/utils/guc.h
@@ -221,14 +221,14 @@ typedef enum
#define GUC_UNIT_KB 0x1000 /* value is in kilobytes */
#define GUC_UNIT_BLOCKS 0x2000 /* value is in blocks */
-#define GUC_UNIT_XBLOCKS 0x3000 /* value is in xlog blocks */
+#define GUC_UNIT_XBLOCKS 0x3000 /* value is in xlog blocks */ //
#define GUC_UNIT_MB 0x4000 /* value is in megabytes */
#define GUC_UNIT_BYTE 0x8000 /* value is in bytes */
#define GUC_UNIT_MEMORY 0xF000 /* mask for size-related units */
#define GUC_UNIT_MS 0x10000 /* value is in milliseconds */
#define GUC_UNIT_S 0x20000 /* value is in seconds */
-#define GUC_UNIT_MIN 0x30000 /* value is in minutes */
+#define GUC_UNIT_MIN 0x30000 /* value is in minutes */ //
#define GUC_UNIT_TIME 0xF0000 /* mask for time-related units */
#define GUC_EXPLAIN 0x100000 /* include in explain */
@@ -239,6 +239,8 @@ typedef enum
*/
#define GUC_RUNTIME_COMPUTED 0x200000
+#define GUC_DYNAMIC_DEFAULT 0x400000 /* default value is dynamic */
+
#define GUC_UNIT (GUC_UNIT_MEMORY | GUC_UNIT_TIME)
--
2.17.1
>From fabe483dc806294eab85fb1dd0690344c5bd1e06 Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horikyota....@gmail.com>
Date: Mon, 30 May 2022 16:11:34 +0900
Subject: [PATCH 2/2] Add fileval-bootval consistency check of GUC parameters
We should keep GUC variables consistent between the default values
written in postgresql.conf.sample and defined in guc.c. Add an
automated way to check for the consistency to the TAP test suite. Some
variables are still excluded since they cannot be checked simple way.
---
src/backend/utils/misc/guc.c | 70 +++++++++++++++++++
src/backend/utils/misc/postgresql.conf.sample | 6 +-
src/include/catalog/pg_proc.dat | 5 ++
src/test/modules/test_misc/t/003_check_guc.pl | 57 +++++++++++++--
4 files changed, 129 insertions(+), 9 deletions(-)
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index dbbadc5a475..a700cbbf4bc 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -7526,6 +7526,76 @@ parse_and_validate_value(struct config_generic *record,
return true;
}
+/*
+ * Helper function for pg_normalize_config_value().
+ * Makes a palloced copy of src then link val to it.
+ * DO NOT destroy val while dst is in use.
+ */
+static struct config_generic *
+copy_config_and_set_value(struct config_generic *src, union config_var_val *val)
+{
+ struct config_generic *dst;
+
+#define CREATE_CONFIG_COPY(dst, src, t) \
+ dst = palloc(sizeof(struct t)); \
+ *(struct t *) dst = *(struct t *) src; \
+
+ switch (src->vartype)
+ {
+ case PGC_BOOL:
+ CREATE_CONFIG_COPY(dst, src, config_bool);
+ ((struct config_bool *)dst)->variable = &val->boolval;
+ break;
+ case PGC_INT:
+ CREATE_CONFIG_COPY(dst, src, config_int);
+ ((struct config_int *)dst)->variable = &val->intval;
+ break;
+ case PGC_REAL:
+ CREATE_CONFIG_COPY(dst, src, config_real);
+ ((struct config_real *)dst)->variable = &val->realval;
+ break;
+ case PGC_STRING:
+ CREATE_CONFIG_COPY(dst, src, config_string);
+ ((struct config_string *)dst)->variable = &val->stringval;
+ break;
+ case PGC_ENUM:
+ CREATE_CONFIG_COPY(dst, src, config_enum);
+ ((struct config_enum *)dst)->variable = &val->enumval;
+ break;
+ }
+
+ return dst;
+}
+
+
+/*
+ * Normalize given value according to the specified GUC variable
+ */
+Datum
+pg_normalize_config_value(PG_FUNCTION_ARGS)
+{
+ char *name = "";
+ char *value = "";
+ struct config_generic *record;
+ char *result;
+ void *extra;
+ union config_var_val altval;
+
+ if (!PG_ARGISNULL(0))
+ name = text_to_cstring(PG_GETARG_TEXT_PP(0));
+ if (!PG_ARGISNULL(1))
+ value = text_to_cstring(PG_GETARG_TEXT_PP(1));
+
+ record = find_option(name, true, false, ERROR);
+
+ parse_and_validate_value(record, name, value, PGC_S_TEST, WARNING,
+ &altval, &extra);
+ record = copy_config_and_set_value(record, &altval);
+
+ result = _ShowOption(record, true);
+
+ PG_RETURN_TEXT_P(cstring_to_text(result));
+}
/*
* Sets option `name' to given value.
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index b4bc06e5f5a..b6403a14496 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -476,7 +476,7 @@
# in all cases.
# These are relevant when logging to syslog:
-#syslog_facility = 'LOCAL0'
+#syslog_facility = 'local0'
#syslog_ident = 'postgres'
#syslog_sequence_numbers = on
#syslog_split_messages = on
@@ -709,7 +709,7 @@
# - Locale and Formatting -
-#datestyle = 'iso, mdy'
+#datestyle = 'ISO, MDY'
#intervalstyle = 'postgres'
#timezone = 'GMT'
#timezone_abbreviations = 'Default' # Select the set of available time zone
@@ -721,7 +721,7 @@
# share/timezonesets/.
#extra_float_digits = 1 # min -15, max 3; any value >0 actually
# selects precise output mode
-#client_encoding = sql_ascii # actually, defaults to database
+#client_encoding = SQL_ASCII # actually, defaults to database
# encoding
# These settings are initialized by initdb, but they can be changed.
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 8873078c160..a92630d768b 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -6118,6 +6118,11 @@
proname => 'pg_settings_get_flags', provolatile => 's', prorettype => '_text',
proargtypes => 'text', prosrc => 'pg_settings_get_flags' },
+{ oid => '9956', descr => 'normalize value to the unit of specified GUC',
+ proname => 'pg_normalize_config_value', proisstrict => 'f',
+ provolatile => 's', prorettype => 'text', proargtypes => 'text text',
+ proargnames => '{varname,value}', prosrc => 'pg_normalize_config_value' },
+
{ oid => '3329', descr => 'show config file settings',
proname => 'pg_show_all_file_settings', prorows => '1000', proretset => 't',
provolatile => 'v', prorettype => 'record', proargtypes => '',
diff --git a/src/test/modules/test_misc/t/003_check_guc.pl b/src/test/modules/test_misc/t/003_check_guc.pl
index 60459ef759e..dfb2d203247 100644
--- a/src/test/modules/test_misc/t/003_check_guc.pl
+++ b/src/test/modules/test_misc/t/003_check_guc.pl
@@ -11,18 +11,40 @@ my $node = PostgreSQL::Test::Cluster->new('main');
$node->init;
$node->start;
+# parameter names that cannot get consistency check performed
+my @ignored_parameters = (
+ 'data_directory',
+ 'hba_file',
+ 'ident_file',
+ 'krb_server_keyfile',
+ 'max_stack_depth',
+ 'bgwriter_flush_after',
+ 'wal_sync_method',
+ 'checkpoint_flush_after',
+ 'timezone_abbreviations',
+ 'lc_messages'
+ );
+
# Grab the names of all the parameters that can be listed in the
# configuration sample file. config_file is an exception, it is not
# in postgresql.conf.sample but is part of the lists from guc.c.
my $all_params = $node->safe_psql(
'postgres',
- "SELECT name
+ "SELECT lower(name), vartype, unit, boot_val, '!'
FROM pg_settings
WHERE NOT 'NOT_IN_SAMPLE' = ANY (pg_settings_get_flags(name)) AND
name <> 'config_file'
ORDER BY 1");
# Note the lower-case conversion, for consistency.
-my @all_params_array = split("\n", lc($all_params));
+my %all_params_hash;
+foreach my $line (split("\n", $all_params))
+{
+ my @f = split('\|', $line);
+ fail("query returned wrong number of columns: $#f : $line") if ($#f != 4);
+ $all_params_hash{$f[0]}->{type} = $f[1];
+ $all_params_hash{$f[0]}->{unit} = $f[2];
+ $all_params_hash{$f[0]}->{bootval} = $f[3];
+}
# Grab the names of all parameters marked as NOT_IN_SAMPLE.
my $not_in_sample = $node->safe_psql(
@@ -43,7 +65,7 @@ my @gucs_in_file;
# Read the sample file line-by-line, checking its contents to build a list
# of everything known as a GUC.
-my $num_tests = 0;
+my @check_elems = ();
open(my $contents, '<', $sample_file)
|| die "Could not open $sample_file: $!";
while (my $line = <$contents>)
@@ -53,11 +75,16 @@ while (my $line = <$contents>)
# file.
# - Valid configuration options are followed immediately by " = ",
# with one space before and after the equal sign.
- if ($line =~ m/^#?([_[:alpha:]]+) = .*/)
+ if ($line =~ m/^#?([_[:alpha:]]+) = (.*)$/)
{
# Lower-case conversion matters for some of the GUCs.
my $param_name = lc($1);
+ # extract value
+ my $file_value = $2;
+ $file_value =~ s/\s*#.*$//; # strip trailing comment
+ $file_value =~ s/^'(.*)'$/$1/; # strip quotes
+
# Ignore some exceptions.
next if $param_name eq "include";
next if $param_name eq "include_dir";
@@ -66,19 +93,37 @@ while (my $line = <$contents>)
# Update the list of GUCs found in the sample file, for the
# follow-up tests.
push @gucs_in_file, $param_name;
+
+ # Check for consistency between bootval and file value.
+ if (!grep { $_ eq $param_name } @ignored_parameters)
+ {
+ push (@check_elems, "('$param_name','$file_value')");
+ }
}
}
close $contents;
+# run consistency check between config-file's default value and boot values.
+my $check_query =
+ 'SELECT f.n, f.v, s.boot_val FROM (VALUES '.
+ join(',', @check_elems).
+ ') f(n,v) JOIN pg_settings s ON s.name = f.n '.
+ 'JOIN pg_settings_get_flags(s.name) flags ON true '.
+ "WHERE 'DYNAMIC_DEFAULT' <> ALL(flags) AND " .
+ 'pg_normalize_config_value(f.n, f.v) <> '.
+ 'pg_normalize_config_value(f.n, s.boot_val)';
+
+is ($node->safe_psql('postgres', $check_query), '',
+ 'check if fileval-bootval consistency is fine');
+
# Cross-check that all the GUCs found in the sample file match the ones
# fetched above. This maps the arrays to a hash, making the creation of
# each exclude and intersection list easier.
my %gucs_in_file_hash = map { $_ => 1 } @gucs_in_file;
-my %all_params_hash = map { $_ => 1 } @all_params_array;
my %not_in_sample_hash = map { $_ => 1 } @not_in_sample_array;
-my @missing_from_file = grep(!$gucs_in_file_hash{$_}, @all_params_array);
+my @missing_from_file = grep(!$gucs_in_file_hash{$_}, keys %all_params_hash);
is(scalar(@missing_from_file),
0, "no parameters missing from postgresql.conf.sample");
--
2.17.1
