On Mon, Feb 13, 2023 at 08:18:52PM -0800, Nathan Bossart wrote:
> On Mon, Feb 13, 2023 at 05:22:45PM -0600, Justin Pryzby wrote:
> > + Reports whether huge pages are in use by the current process.
> > + See <xref linkend="guc-huge-pages"/> for more information.
>
> nitpick: Should this say "server" instead of "current process"?
It should probably say "instance" :)
> > +static char *huge_pages_active = "unknown"; /* dynamically set */
>
> nitpick: Does this need to be initialized here?
None of the GUCs' C vars need to be initialized, since the guc machinery
will do it.
...but the convention is that they *are* initialized - and that's now
partially enforced.
See:
d9d873bac67047cfacc9f5ef96ee488f2cb0f1c3
7d25958453a60337bcb7bcc986e270792c007ea4
a73952b795632b2cf5acada8476e7cf75857e9be
> > + {
> > + {"huge_pages_active", PGC_INTERNAL, PRESET_OPTIONS,
> > + gettext_noop("Indicates whether huge pages are in
> > use."),
> > + NULL,
> > + GUC_NOT_IN_SAMPLE | GUC_DISALLOW_IN_FILE |
> > GUC_RUNTIME_COMPUTED
> > + },
> > + &huge_pages_active,
> > + "unknown",
> > + NULL, NULL, NULL
> > + },
>
> I'm curious why you chose to make this a string instead of an enum. There
> might be little practical difference, but since there are only three
> possible values, I wonder if it'd be better form to make it an enum.
It takes more code to write as an enum - see 002.txt. I'm not convinced
this is better.
But your comment made me fix its <type>, and reconsider the strings,
which I changed to active={unknown/true/false} rather than {unk/on/off}.
It could also be active={unknown/yes/no}...
--
Justin
>From 7bf67d398902570ffc68a6b78265a0e346503392 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <[email protected]>
Date: Mon, 23 Jan 2023 18:33:51 -0600
Subject: [PATCH v4 1/2] add GUC: huge_pages_active
This is useful to show the current state of huge pages when
huge_pages=try. The effective status is not otherwise visible without
OS level tools like gdb or /proc/N/smaps.
https://www.postgresql.org/message-id/flat/tu4pr8401mb1152ebb0d271f827e2e37a01ee...@tu4pr8401mb1152.namprd84.prod.outlook.com
---
doc/src/sgml/config.sgml | 17 ++++++++++++++++-
src/backend/port/sysv_shmem.c | 3 +++
src/backend/port/win32_shmem.c | 4 ++++
src/backend/utils/misc/guc_tables.c | 12 ++++++++++++
4 files changed, 35 insertions(+), 1 deletion(-)
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 8c56b134a84..3ff301edf8a 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -1700,23 +1700,24 @@ include_dir 'conf.d'
</indexterm>
</term>
<listitem>
<para>
Controls whether huge pages are requested for the main shared memory
area. Valid values are <literal>try</literal> (the default),
<literal>on</literal>, and <literal>off</literal>. With
<varname>huge_pages</varname> set to <literal>try</literal>, the
server will try to request huge pages, but fall back to the default if
that fails. With <literal>on</literal>, failure to request huge pages
will prevent the server from starting up. With <literal>off</literal>,
- huge pages will not be requested.
+ huge pages will not be requested. The actual state of huge pages is
+ indicated by the server variable <xref linkend="guc-huge-pages-active"/>.
</para>
<para>
At present, this setting is supported only on Linux and Windows. The
setting is ignored on other systems when set to
<literal>try</literal>. On Linux, it is only supported when
<varname>shared_memory_type</varname> is set to <literal>mmap</literal>
(the default).
</para>
<para>
@@ -10679,22 +10680,36 @@ dynamic_library_path = 'C:\tools\postgresql;H:\my_project\lib;$libdir'
with assertions enabled. That is the case if the
macro <symbol>USE_ASSERT_CHECKING</symbol> is defined
when <productname>PostgreSQL</productname> is built (accomplished
e.g., by the <command>configure</command> option
<option>--enable-cassert</option>). By
default <productname>PostgreSQL</productname> is built without
assertions.
</para>
</listitem>
</varlistentry>
+ <varlistentry id="guc-huge-pages-active" xreflabel="huge_pages_active">
+ <term><varname>huge_pages_active</varname> (<type>string</type>)
+ <indexterm>
+ <primary><varname>huge_pages_active</varname> configuration parameter</primary>
+ </indexterm>
+ </term>
+ <listitem>
+ <para>
+ Reports whether huge pages are in use by the current instance.
+ See <xref linkend="guc-huge-pages"/> for more information.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry id="guc-integer-datetimes" xreflabel="integer_datetimes">
<term><varname>integer_datetimes</varname> (<type>boolean</type>)
<indexterm>
<primary><varname>integer_datetimes</varname> configuration parameter</primary>
</indexterm>
</term>
<listitem>
<para>
Reports whether <productname>PostgreSQL</productname> was built with support for
64-bit-integer dates and times. As of <productname>PostgreSQL</productname> 10,
this is always <literal>on</literal>.
diff --git a/src/backend/port/sysv_shmem.c b/src/backend/port/sysv_shmem.c
index eaba244bc9c..89ec5c30364 100644
--- a/src/backend/port/sysv_shmem.c
+++ b/src/backend/port/sysv_shmem.c
@@ -619,22 +619,25 @@ CreateAnonymousSegment(Size *size)
allocsize += hugepagesize - (allocsize % hugepagesize);
ptr = mmap(NULL, allocsize, PROT_READ | PROT_WRITE,
PG_MMAP_FLAGS | mmap_flags, -1, 0);
mmap_errno = errno;
if (huge_pages == HUGE_PAGES_TRY && ptr == MAP_FAILED)
elog(DEBUG1, "mmap(%zu) with MAP_HUGETLB failed, huge pages disabled: %m",
allocsize);
}
#endif
+ SetConfigOption("huge_pages_active", ptr == MAP_FAILED ? "false" : "true",
+ PGC_INTERNAL, PGC_S_DYNAMIC_DEFAULT);
+
if (ptr == MAP_FAILED && huge_pages != HUGE_PAGES_ON)
{
/*
* Use the original size, not the rounded-up value, when falling back
* to non-huge pages.
*/
allocsize = *size;
ptr = mmap(NULL, allocsize, PROT_READ | PROT_WRITE,
PG_MMAP_FLAGS, -1, 0);
mmap_errno = errno;
}
diff --git a/src/backend/port/win32_shmem.c b/src/backend/port/win32_shmem.c
index 62e08074770..fa72b3d1f8f 100644
--- a/src/backend/port/win32_shmem.c
+++ b/src/backend/port/win32_shmem.c
@@ -319,22 +319,26 @@ retry:
* If the segment already existed, CreateFileMapping() will return a
* handle to the existing one and set ERROR_ALREADY_EXISTS.
*/
if (GetLastError() == ERROR_ALREADY_EXISTS)
{
CloseHandle(hmap); /* Close the handle, since we got a valid one
* to the previous segment. */
hmap = NULL;
Sleep(1000);
continue;
}
+
+ SetConfigOption("huge_pages_active", (flProtect & SEC_LARGE_PAGES) ?
+ "true" : "false", PGC_INTERNAL, PGC_S_DYNAMIC_DEFAULT);
+
break;
}
/*
* If the last call in the loop still returned ERROR_ALREADY_EXISTS, this
* shared memory segment exists and we assume it belongs to somebody else.
*/
if (!hmap)
ereport(FATAL,
(errmsg("pre-existing shared memory block is still in use"),
errhint("Check if there are any old server processes still running, and terminate them.")));
diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c
index b21698934c8..1a298f16c81 100644
--- a/src/backend/utils/misc/guc_tables.c
+++ b/src/backend/utils/misc/guc_tables.c
@@ -554,22 +554,23 @@ static int server_version_num;
#define DEFAULT_SYSLOG_FACILITY LOG_LOCAL0
#else
#define DEFAULT_SYSLOG_FACILITY 0
#endif
static int syslog_facility = DEFAULT_SYSLOG_FACILITY;
static char *timezone_string;
static char *log_timezone_string;
static char *timezone_abbreviations_string;
static char *data_directory;
static char *session_authorization_string;
+static char *huge_pages_active = "unknown"; /* dynamically set */
static int max_function_args;
static int max_index_keys;
static int max_identifier_length;
static int block_size;
static int segment_size;
static int shared_memory_size_mb;
static int shared_memory_size_in_huge_pages;
static int wal_block_size;
static bool data_checksums;
static bool integer_datetimes;
@@ -4516,22 +4517,33 @@ struct config_string ConfigureNamesString[] =
{
{"backtrace_functions", PGC_SUSET, DEVELOPER_OPTIONS,
gettext_noop("Log backtrace for errors in these functions."),
NULL,
GUC_NOT_IN_SAMPLE
},
&backtrace_functions,
"",
check_backtrace_functions, assign_backtrace_functions, NULL
},
+ {
+ {"huge_pages_active", PGC_INTERNAL, PRESET_OPTIONS,
+ gettext_noop("Indicates whether huge pages are in use."),
+ NULL,
+ GUC_NOT_IN_SAMPLE | GUC_DISALLOW_IN_FILE | GUC_RUNTIME_COMPUTED
+ },
+ &huge_pages_active,
+ "unknown",
+ NULL, NULL, NULL
+ },
+
/* End-of-list marker */
{
{NULL, 0, 0, NULL, NULL}, NULL, NULL, NULL, NULL, NULL
}
};
struct config_enum ConfigureNamesEnum[] =
{
{
{"backslash_quote", PGC_USERSET, COMPAT_OPTIONS_PREVIOUS,
--
2.34.1
>From 96ad5dcd77bb8d2e4c9c3ca06ae4dcf43017defe Mon Sep 17 00:00:00 2001
From: Justin Pryzby <[email protected]>
Date: Tue, 14 Feb 2023 15:29:41 -0600
Subject: [PATCH v4 2/2] f! convert to an enum
---
doc/src/sgml/config.sgml | 2 +-
src/backend/utils/misc/guc_tables.c | 32 ++++++++++++++++++-----------
src/include/storage/pg_shmem.h | 8 ++++++++
3 files changed, 29 insertions(+), 13 deletions(-)
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 3ff301edf8a..1f6f71a2826 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -10681,23 +10681,23 @@ dynamic_library_path = 'C:\tools\postgresql;H:\my_project\lib;$libdir'
macro <symbol>USE_ASSERT_CHECKING</symbol> is defined
when <productname>PostgreSQL</productname> is built (accomplished
e.g., by the <command>configure</command> option
<option>--enable-cassert</option>). By
default <productname>PostgreSQL</productname> is built without
assertions.
</para>
</listitem>
</varlistentry>
<varlistentry id="guc-huge-pages-active" xreflabel="huge_pages_active">
- <term><varname>huge_pages_active</varname> (<type>string</type>)
+ <term><varname>huge_pages_active</varname> (<type>enum</type>)
<indexterm>
<primary><varname>huge_pages_active</varname> configuration parameter</primary>
</indexterm>
</term>
<listitem>
<para>
Reports whether huge pages are in use by the current instance.
See <xref linkend="guc-huge-pages"/> for more information.
</para>
</listitem>
</varlistentry>
diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c
index 1a298f16c81..f9c98997492 100644
--- a/src/backend/utils/misc/guc_tables.c
+++ b/src/backend/utils/misc/guc_tables.c
@@ -339,22 +339,29 @@ static const struct config_enum_entry huge_pages_options[] = {
{"on", HUGE_PAGES_ON, false},
{"try", HUGE_PAGES_TRY, false},
{"true", HUGE_PAGES_ON, true},
{"false", HUGE_PAGES_OFF, true},
{"yes", HUGE_PAGES_ON, true},
{"no", HUGE_PAGES_OFF, true},
{"1", HUGE_PAGES_ON, true},
{"0", HUGE_PAGES_OFF, true},
{NULL, 0, false}
};
+static const struct config_enum_entry huge_pages_active_options[] = {
+ {"unknown", HUGE_PAGES_ACTIVE_UNKNOWN, false},
+ {"false", HUGE_PAGES_ACTIVE_FALSE, false},
+ {"true", HUGE_PAGES_ACTIVE_TRUE, false},
+ {NULL, 0, false}
+};
+
static const struct config_enum_entry recovery_prefetch_options[] = {
{"off", RECOVERY_PREFETCH_OFF, false},
{"on", RECOVERY_PREFETCH_ON, false},
{"try", RECOVERY_PREFETCH_TRY, false},
{"true", RECOVERY_PREFETCH_ON, true},
{"false", RECOVERY_PREFETCH_OFF, true},
{"yes", RECOVERY_PREFETCH_ON, true},
{"no", RECOVERY_PREFETCH_OFF, true},
{"1", RECOVERY_PREFETCH_ON, true},
{"0", RECOVERY_PREFETCH_OFF, true},
{NULL, 0, false}
@@ -527,22 +534,24 @@ int tcp_user_timeout;
* renegotiation and therefore always try to zero it.
*/
int ssl_renegotiation_limit;
/*
* This really belongs in pg_shmem.c, but is defined here so that it doesn't
* need to be duplicated in all the different implementations of pg_shmem.c.
*/
int huge_pages = HUGE_PAGES_TRY;
int huge_page_size;
+int huge_pages_active = HUGE_PAGES_ACTIVE_UNKNOWN;
+
/*
* These variables are all dummies that don't do anything, except in some
* cases provide the value for SHOW to display. The real state is elsewhere
* and is kept in sync by assign_hooks.
*/
static char *syslog_ident_str;
static double phony_random_seed;
static char *client_encoding_string;
static char *datestyle_string;
static char *locale_collate;
static char *locale_ctype;
@@ -554,23 +563,22 @@ static int server_version_num;
#define DEFAULT_SYSLOG_FACILITY LOG_LOCAL0
#else
#define DEFAULT_SYSLOG_FACILITY 0
#endif
static int syslog_facility = DEFAULT_SYSLOG_FACILITY;
static char *timezone_string;
static char *log_timezone_string;
static char *timezone_abbreviations_string;
static char *data_directory;
static char *session_authorization_string;
-static char *huge_pages_active = "unknown"; /* dynamically set */
static int max_function_args;
static int max_index_keys;
static int max_identifier_length;
static int block_size;
static int segment_size;
static int shared_memory_size_mb;
static int shared_memory_size_in_huge_pages;
static int wal_block_size;
static bool data_checksums;
static bool integer_datetimes;
@@ -4517,33 +4525,22 @@ struct config_string ConfigureNamesString[] =
{
{"backtrace_functions", PGC_SUSET, DEVELOPER_OPTIONS,
gettext_noop("Log backtrace for errors in these functions."),
NULL,
GUC_NOT_IN_SAMPLE
},
&backtrace_functions,
"",
check_backtrace_functions, assign_backtrace_functions, NULL
},
- {
- {"huge_pages_active", PGC_INTERNAL, PRESET_OPTIONS,
- gettext_noop("Indicates whether huge pages are in use."),
- NULL,
- GUC_NOT_IN_SAMPLE | GUC_DISALLOW_IN_FILE | GUC_RUNTIME_COMPUTED
- },
- &huge_pages_active,
- "unknown",
- NULL, NULL, NULL
- },
-
/* End-of-list marker */
{
{NULL, 0, 0, NULL, NULL}, NULL, NULL, NULL, NULL, NULL
}
};
struct config_enum ConfigureNamesEnum[] =
{
{
{"backslash_quote", PGC_USERSET, COMPAT_OPTIONS_PREVIOUS,
@@ -4845,22 +4842,33 @@ struct config_enum ConfigureNamesEnum[] =
{
{"huge_pages", PGC_POSTMASTER, RESOURCES_MEM,
gettext_noop("Use of huge pages on Linux or Windows."),
NULL
},
&huge_pages,
HUGE_PAGES_TRY, huge_pages_options,
NULL, NULL, NULL
},
+ {
+ {"huge_pages_active", PGC_INTERNAL, PRESET_OPTIONS,
+ gettext_noop("Indicates whether huge pages are in use."),
+ NULL,
+ GUC_NOT_IN_SAMPLE | GUC_DISALLOW_IN_FILE | GUC_RUNTIME_COMPUTED
+ },
+ &huge_pages_active,
+ HUGE_PAGES_ACTIVE_UNKNOWN, huge_pages_active_options,
+ NULL, NULL, NULL
+ },
+
{
{"recovery_prefetch", PGC_SIGHUP, WAL_RECOVERY,
gettext_noop("Prefetch referenced blocks during recovery."),
gettext_noop("Look ahead in the WAL to find references to uncached data.")
},
&recovery_prefetch,
RECOVERY_PREFETCH_TRY, recovery_prefetch_options,
check_recovery_prefetch, assign_recovery_prefetch, NULL
},
{
diff --git a/src/include/storage/pg_shmem.h b/src/include/storage/pg_shmem.h
index 4dd05f156d5..1699bf64238 100644
--- a/src/include/storage/pg_shmem.h
+++ b/src/include/storage/pg_shmem.h
@@ -46,22 +46,30 @@ extern PGDLLIMPORT int shared_memory_type;
extern PGDLLIMPORT int huge_pages;
extern PGDLLIMPORT int huge_page_size;
/* Possible values for huge_pages */
typedef enum
{
HUGE_PAGES_OFF,
HUGE_PAGES_ON,
HUGE_PAGES_TRY
} HugePagesType;
+/* Possible values for huge_pages_active */
+typedef enum
+{
+ HUGE_PAGES_ACTIVE_UNKNOWN,
+ HUGE_PAGES_ACTIVE_FALSE,
+ HUGE_PAGES_ACTIVE_TRUE,
+} HugePagesActiveType;
+
/* Possible values for shared_memory_type */
typedef enum
{
SHMEM_TYPE_WINDOWS,
SHMEM_TYPE_SYSV,
SHMEM_TYPE_MMAP
} PGShmemType;
#ifndef WIN32
extern PGDLLIMPORT unsigned long UsedShmemSegID;
#else
--
2.34.1
