From: Michael Paquier <michael@paquier.xyz>
To: Justin Pryzby <pryzby@telsasoft.com>
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: Tue, 28 Mar 2023 09:35:30 +0900
Message-ID: <ZCI2UlFUxFz80otI@paquier.xyz> (raw)
In-Reply-To: <ZB0B+u5RMN8g52kF@telsasoft.com>
References: <ZBfsbTEUnRcEyNao@paquier.xyz>
<ZBplvQx7n0MGVr6O@telsasoft.com>
<ZBqwHY7yZQATAp9N@paquier.xyz>
<ZBt+tCS/Akj8BvHi@telsasoft.com>
<ZBt/4EdbwLp2hJLx@paquier.xyz>
<ZB0B+u5RMN8g52kF@telsasoft.com>
On Thu, Mar 23, 2023 at 08:50:50PM -0500, Justin Pryzby wrote:
> 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.
Yeah, I kind of agree that the complications are not appealing
compared to the use case. FWIW, I am fine with just using "unknown"
even under -C because we don't know the status without the mmpa()
call. And we don't want to assign what would be potentially a bunch
of memory when running that.
> 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 ?
huge_pages_status is OK here, as is reusing the enum struct to avoid
an unnecessary duplication.
> I mis-used cirrusci to check that the GUC does work correctly under
> windows.
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?
> 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.
+++ 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?
+ <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.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (832B, ../ZCI2UlFUxFz80otI@paquier.xyz/2-signature.asc)
download
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: michael@paquier.xyz, pryzby@telsasoft.com, 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: <ZCI2UlFUxFz80otI@paquier.xyz>
* 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