On Thu, Mar 23, 2023 at 05:25:46PM +0900, Kyotaro Horiguchi wrote:
> Wouldn't storing the value in the shared memory itself work? Though,
> that space does become almost dead for the server's lifetime...
I'm sure it's possible, but it's also not worth writing a special
implementation just to handle huge_pages_active, which is better written
in 30 lines than in 300 lines.
If we needed to avoid using a GUC, maybe it'd work to add
huge_pages_active to PGShmemHeader. But "avoid using gucs at all costs"
isn't the goal, and using a GUC parallels all the similar, and related
things without needing to allocate extra bits of shared_memory.
On Thu, Mar 23, 2023 at 07:23:28AM +0900, Michael Paquier wrote:
> On Wed, Mar 22, 2023 at 05:18:28PM -0500, Justin Pryzby wrote:
> > Wow, good point. I think to make it work we'd need put
> > huge_pages_active into BackendParameters and handle it in
> > save_backend_variables(). If so, that'd be strong argument for using a
> > GUC, which already has all the necessary infrastructure for exposing the
> > server's state.
>
> I am afraid so, duplicating an existing infrastructure for a need like
> the one of this thread is not really appealing.
This goes back to using a GUC, and:
- removes GUC_RUNTIME_COMPUTED, since that causes a useless error when
using -C if the server is running, rather than successfully printing
"unknown".
- I also renamed it from huge_pages_active = {true,false,unknown} to
huge_pages_STATUS = {on,off,unknown}. This parallels huge_pages,
which is documented to accept values on/off/try. Also, true/false
isn't how bools are displayed.
- I realized that the rename suggested implementing it as an enum GUC,
and re-using the existing HUGE_PAGES_{ON,OFF} values (and adding an
"UNKNOWN"). Maybe this also avoids Stephen's earlier objection to
using a string ?
I mis-used cirrusci to check that the GUC does work correctly under
windows.
If there's continuing aversions to using a GUC, please say so, and try
to suggest an alternate implementation you think would be justified.
It'd need to work under windows with EXEC_BACKEND.
--
Justin
>From 951d481b74ac978482b9d5794b437f741518f3a9 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <[email protected]>
Date: Mon, 23 Jan 2023 18:33:51 -0600
Subject: [PATCH v6] 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 | 20 ++++++++++++++++++++
src/include/storage/pg_shmem.h | 5 +++--
5 files changed, 46 insertions(+), 3 deletions(-)
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 510ff88fc5e..88998238645 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -1708,7 +1708,8 @@ include_dir 'conf.d'
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-status"/>.
</para>
<para>
@@ -10660,6 +10661,20 @@ dynamic_library_path = 'C:\tools\postgresql;H:\my_project\lib;$libdir'
</listitem>
</varlistentry>
+ <varlistentry id="guc-huge-pages-status" xreflabel="huge_pages_status">
+ <term><varname>huge_pages_status</varname> (<type>enum</type>)
+ <indexterm>
+ <primary><varname>huge_pages_status</varname> configuration parameter</primary>
+ </indexterm>
+ </term>
+ <listitem>
+ <para>
+ Reports the state of huge pages in 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>
diff --git a/src/backend/port/sysv_shmem.c b/src/backend/port/sysv_shmem.c
index eaba244bc9c..b6104c6ce60 100644
--- a/src/backend/port/sysv_shmem.c
+++ b/src/backend/port/sysv_shmem.c
@@ -627,6 +627,9 @@ CreateAnonymousSegment(Size *size)
}
#endif
+ SetConfigOption("huge_pages_status", ptr == MAP_FAILED ? "off" : "on",
+ PGC_INTERNAL, PGC_S_DYNAMIC_DEFAULT);
+
if (ptr == MAP_FAILED && huge_pages != HUGE_PAGES_ON)
{
/*
diff --git a/src/backend/port/win32_shmem.c b/src/backend/port/win32_shmem.c
index 62e08074770..199f2e23a13 100644
--- a/src/backend/port/win32_shmem.c
+++ b/src/backend/port/win32_shmem.c
@@ -327,6 +327,10 @@ retry:
Sleep(1000);
continue;
}
+
+ SetConfigOption("huge_pages_status", (flProtect & SEC_LARGE_PAGES) ?
+ "on" : "off", PGC_INTERNAL, PGC_S_DYNAMIC_DEFAULT);
+
break;
}
diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c
index 8696d965dee..785bee948e9 100644
--- a/src/backend/utils/misc/guc_tables.c
+++ b/src/backend/utils/misc/guc_tables.c
@@ -348,6 +348,13 @@ static const struct config_enum_entry huge_pages_options[] = {
{NULL, 0, false}
};
+static const struct config_enum_entry huge_pages_status_options[] = {
+ {"off", HUGE_PAGES_OFF, false},
+ {"on", HUGE_PAGES_ON, false},
+ {"unknown", HUGE_PAGES_UNKNOWN, false},
+ {NULL, 0, false}
+};
+
static const struct config_enum_entry recovery_prefetch_options[] = {
{"off", RECOVERY_PREFETCH_OFF, false},
{"on", RECOVERY_PREFETCH_ON, false},
@@ -536,6 +543,8 @@ int ssl_renegotiation_limit;
int huge_pages = HUGE_PAGES_TRY;
int huge_page_size;
+int huge_pages_status = HUGE_PAGES_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
@@ -4833,6 +4842,17 @@ struct config_enum ConfigureNamesEnum[] =
NULL, NULL, NULL
},
+ {
+ {"huge_pages_status", PGC_INTERNAL, PRESET_OPTIONS,
+ gettext_noop("Indicates the status of huge pages."),
+ NULL,
+ GUC_NOT_IN_SAMPLE | GUC_DISALLOW_IN_FILE
+ },
+ &huge_pages_status,
+ HUGE_PAGES_UNKNOWN, huge_pages_status_options,
+ NULL, NULL, NULL
+ },
+
{
{"recovery_prefetch", PGC_SIGHUP, WAL_RECOVERY,
gettext_noop("Prefetch referenced blocks during recovery."),
diff --git a/src/include/storage/pg_shmem.h b/src/include/storage/pg_shmem.h
index 4dd05f156d5..bfda42b0b26 100644
--- a/src/include/storage/pg_shmem.h
+++ b/src/include/storage/pg_shmem.h
@@ -46,12 +46,13 @@ extern PGDLLIMPORT int shared_memory_type;
extern PGDLLIMPORT int huge_pages;
extern PGDLLIMPORT int huge_page_size;
-/* Possible values for huge_pages */
+/* Possible values for huge_pages/huge_pages_status */
typedef enum
{
HUGE_PAGES_OFF,
HUGE_PAGES_ON,
- HUGE_PAGES_TRY
+ HUGE_PAGES_TRY, /* only for huge_pages */
+ HUGE_PAGES_UNKNOWN /* only for huge_pages_status */
} HugePagesType;
/* Possible values for shared_memory_type */
--
2.34.1
