From: Justin Pryzby <pryzby@telsasoft.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Cc: sfrost@snowman.net
Cc: alvherre@alvh.no-ip.org
Cc: andres@anarazel.de
Cc: nathandbossart@gmail.com
Cc: jchampion@timescale.com
Cc: john.naylor@enterprisedb.com
Cc: masao.fujii@oss.nttdata.com
Cc: noriyoshi.shinoda@hpe.com
Cc: pgsql-hackers@postgresql.org
Cc: rjuju123@gmail.com
Cc: sawada.mshk@gmail.com
Cc: tgl@sss.pgh.pa.us
Cc: thomas.munro@gmail.com
Subject: Re: Improve logging when using Huge Pages
Date: Thu, 6 Apr 2023 16:57:58 -0500
Message-ID: <ZC9AZqCIAxjTEoRk@telsasoft.com> (raw)
In-Reply-To: <ZCI2UlFUxFz80otI@paquier.xyz>
References: <ZBfsbTEUnRcEyNao@paquier.xyz>
<ZBplvQx7n0MGVr6O@telsasoft.com>
<ZBqwHY7yZQATAp9N@paquier.xyz>
<ZBt+tCS/Akj8BvHi@telsasoft.com>
<ZBt/4EdbwLp2hJLx@paquier.xyz>
<ZB0B+u5RMN8g52kF@telsasoft.com>
<ZCI2UlFUxFz80otI@paquier.xyz>
On Tue, Mar 28, 2023 at 09:35:30AM +0900, Michael Paquier wrote:
> On Thu, Mar 23, 2023 at 08:50:50PM -0500, Justin Pryzby wrote:
>
> You mean that you abused of it in some custom branch running the CI on
> github? If I may ask, what did you do to make sure that huge pages
> are set when re-attaching a backend to a shmem area?
Yes. I hijacked a tap test to first run "show huge_pages_active" and then
also caused it to fail, such that I could check its output.
https://cirrus-ci.com/task/6309510885670912https://cirrus-ci.com/task/6613427737591808> > 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.
>
> Looking at the patch, it seems like that this should work even for
> EXEC_BACKEND on *nix when a backend reattaches.. It may be better to
> be sure of that, as well, if it has not been tested.
Arg, no - it wasn't working. I added an assert to check that a running
server won't output "unknown".
> +++ 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)
>
> Perhaps better to just move that at the end of PGSharedMemoryCreate()
> for WIN32?
Maybe - I don't know.
> + <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>
>
> The patch needs to provide more details about the unknown state, IMO,
> to make it crystal-clear to the users what are the limitations of this
> GUC, especially regarding the fact that this is useful when "try"-ing
> to allocate huge pages, and that the value can only be consulted after
> the server has been started.
I'm not sure I agree. It can be confusing (even harmful) to overspecify the
documentation. I used the word "instance" specifically to refer to a running
server. Anyone querying the status of huge pages is going to need to
understand that it doesn't make sense to ask about the memory state of a server
that's not running. Actually, I'm having trouble imagining that anybody would
ever run postgres -C huge_page_status except if they wondered whether it might
misbehave. If what I've written is inadequate, could you propose something ?
--
Justin
PS. I hadn't updated this thread because my last message from you (on
another thread) indicated that you'd gotten busy. I'm hoping you'll
respond to these other inquiries when you're able.
https://www.postgresql.org/message-id/ZCUZLiCeXq4Im7OG%40telsasoft.comhttps://www.postgresql.org/message-id/ZCUKL22GutwGrrZk%40telsasoft.comAttachments:
[text/x-diff] v7-0001-add-GUC-huge_pages_active.patch (6.2K, ../ZC9AZqCIAxjTEoRk@telsasoft.com/2-v7-0001-add-GUC-huge_pages_active.patch)
download | inline diff:
From 078724d0ec411de1c52cb9a43a3a29c644a8d19a Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Mon, 23 Jan 2023 18:33:51 -0600
Subject: [PATCH v7] 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/TU4PR8401MB1152EBB0D271F827E2E37A01EECC9@TU4PR8401MB1152.NAMPRD84.PROD.OUTLOOK.COM
---
doc/src/sgml/config.sgml | 21 ++++++++++++++++++++-
src/backend/port/sysv_shmem.c | 7 +++++++
src/backend/port/win32_shmem.c | 4 ++++
src/backend/storage/ipc/ipci.c | 2 ++
src/backend/utils/misc/guc_tables.c | 20 ++++++++++++++++++++
src/include/storage/pg_shmem.h | 5 +++--
6 files changed, 56 insertions(+), 3 deletions(-)
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgmlindex 096be7cb8cc..de74d3d1a81 100644--- a/doc/src/sgml/config.sgml+++ b/doc/src/sgml/config.sgml@@ -1728,7 +1728,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>
@@ -10710,6 +10711,24 @@ 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:+ <literal>on</literal>, <literal>off</literal>, or (if displayed with+ <literal>postgres -C</literal>) <literal>unknown</literal>.+ This parameter is useful to determine whether allocation of huge pages+ was successful when <literal>huge_pages=try</literal>.+ 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.cindex eaba244bc9c..a958f75f7a4 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)
{
/*
@@ -737,7 +740,11 @@ PGSharedMemoryCreate(Size size,
sysvsize = sizeof(PGShmemHeader);
}
else
+ {
sysvsize = size;
+ SetConfigOption("huge_pages_status", "off",+ PGC_INTERNAL, PGC_S_DYNAMIC_DEFAULT);+ }
/*
* Loop till we find a free IPC key. Trust CreateDataDirLockFile() to
diff --git a/src/backend/port/win32_shmem.c b/src/backend/port/win32_shmem.cindex 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/storage/ipc/ipci.c b/src/backend/storage/ipc/ipci.cindex 8f1ded7338f..f760675691a 100644--- a/src/backend/storage/ipc/ipci.c+++ b/src/backend/storage/ipc/ipci.c@@ -190,6 +190,8 @@ CreateSharedMemoryAndSemaphores(void)
*/
seghdr = PGSharedMemoryCreate(size, &shim);
+ Assert(strcmp("unknown", GetConfigOption("huge_pages_status", false, false)) != 0);+
InitShmemAccess(seghdr);
/*
diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.cindex 0a399afa8ca..a6b7fd28e68 100644--- a/src/backend/utils/misc/guc_tables.c+++ b/src/backend/utils/misc/guc_tables.c@@ -366,6 +366,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},
@@ -554,6 +561,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
@@ -4889,6 +4898,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.hindex 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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: pryzby@telsasoft.com, michael@paquier.xyz, horikyota.ntt@gmail.com, sfrost@snowman.net, alvherre@alvh.no-ip.org, andres@anarazel.de, nathandbossart@gmail.com, jchampion@timescale.com, john.naylor@enterprisedb.com, masao.fujii@oss.nttdata.com, noriyoshi.shinoda@hpe.com, rjuju123@gmail.com, sawada.mshk@gmail.com, tgl@sss.pgh.pa.us, thomas.munro@gmail.com
Subject: Re: Improve logging when using Huge Pages
In-Reply-To: <ZC9AZqCIAxjTEoRk@telsasoft.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox