public inbox for [email protected]
help / color / mirror / Atom feedFrom: Justin Pryzby <[email protected]>
To: Stephen Frost <[email protected]>
Cc: Nathan Bossart <[email protected]>
Cc: Alvaro Herrera <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: Michael Paquier <[email protected]>
Cc: Kyotaro Horiguchi <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Subject: Re: Improve logging when using Huge Pages
Date: Mon, 13 Mar 2023 15:03:50 -0500
Message-ID: <ZA+Bpk/[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
<20230209192909.GA606606@nathanxps13>
<[email protected]>
<20230214041852.GA1190218@nathanxps13>
<[email protected]>
<20230215181317.GA1353037@nathanxps13>
<20230308221656.GA3621885@nathanxps13>
<ZAnuYi/[email protected]>
<ZAoLoCPRf6j/[email protected]>
<[email protected]>
On Thu, Mar 09, 2023 at 03:02:29PM -0500, Stephen Frost wrote:
> * Justin Pryzby ([email protected]) wrote:
> > On Thu, Mar 09, 2023 at 09:34:10AM -0500, Stephen Frost wrote:
> > > * Nathan Bossart ([email protected]) wrote:
> > > > On Wed, Feb 15, 2023 at 10:13:17AM -0800, Nathan Bossart wrote:
> > > > > On Tue, Feb 14, 2023 at 07:32:56PM -0600, Justin Pryzby wrote:
> > > > >> On Mon, Feb 13, 2023 at 08:18:52PM -0800, Nathan Bossart wrote:
> > > > >>> 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}...
> > > > >
> > > > > I think unknown/true/false is fine. I'm okay with using a string if no one
> > > > > else thinks it should be an enum (or a bool).
> > > >
> > > > There's been no response for this, so I guess we can proceed with a string
> > > > GUC.
> > >
> > > Just happened to see this and I'm not really a fan of this being a
> > > string when it's pretty clear that's not what it actually is.
> >
> > I don't understand what you mean by that.
> > Why do you say it isn't a string ?
>
> Because it's clearly a yes/no, either the server is currently running
> with huge pages, or it isn't. That's the definition of a boolean.
I originally implemented it as a boolean, and I changed it in response
to the feedback that postgres -C huge_pages_active should return
"unknown".
> > Is there an agreement to use a function, instead ?
Alvaro was -1 on using a function
Andres is +0 (?)
Nathan is +1
Stephen is +1
I'm -0.5, but I reimplemented it as a function. I hope that helps it to
progress. Please include a suggestion if there's better place for the
function or global var.
--
Justin
Attachments:
[text/x-diff] v5-0001-add-pg_huge_pages_active.patch (8.6K, ../ZA+Bpk%[email protected]/2-v5-0001-add-pg_huge_pages_active.patch)
download | inline diff:
From cd171da2150e1ee8cfc6ca4bdee0a591df6f566b Mon Sep 17 00:00:00 2001
From: Justin Pryzby <[email protected]>
Date: Mon, 23 Jan 2023 18:33:51 -0600
Subject: [PATCH v5] add pg_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 | 3 ++-
doc/src/sgml/func.sgml | 14 ++++++++++++++
src/backend/port/sysv_shmem.c | 2 ++
src/backend/port/win32_shmem.c | 2 ++
src/backend/utils/adt/misc.c | 11 +++++++++++
src/include/catalog/pg_proc.dat | 5 +++++
src/include/miscadmin.h | 3 +++
7 files changed, 39 insertions(+), 1 deletion(-)
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index e5c41cc6c6d..d66f73a494a 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 <function>pg_huge_pages_active()</function>.
</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>
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index a5bf2f01b57..4e99d5aff5c 100644
--- a/doc/src/sgml/func.sgml
+++ b/doc/src/sgml/func.sgml
@@ -22516,22 +22516,36 @@ SELECT * FROM pg_ls_dir('.') WITH ORDINALITY AS t(ls,n);
To request information about a specific log file format, supply
either <literal>csvlog</literal>, <literal>jsonlog</literal> or
<literal>stderr</literal> as the
value of the optional parameter. The result is <literal>NULL</literal>
if the log format requested is not configured in
<xref linkend="guc-log-destination"/>.
The result reflects the contents of
the <filename>current_logfiles</filename> file.
</para></entry>
</row>
+ <row>
+ <entry role="func_table_entry"><para role="func_signature">
+ <indexterm>
+ <primary>pg_huge_pages_active</primary>
+ </indexterm>
+ <function>pg_huge_pages_active</function> ()
+ <returnvalue>bool</returnvalue>
+ </para>
+ <para>
+ Reports whether huge pages are in use by the current instance.
+ See <xref linkend="guc-huge-pages"/> for more information.
+ </para></entry>
+ </row>
+
<row>
<entry role="func_table_entry"><para role="func_signature">
<indexterm>
<primary>pg_my_temp_schema</primary>
</indexterm>
<function>pg_my_temp_schema</function> ()
<returnvalue>oid</returnvalue>
</para>
<para>
Returns the OID of the current session's temporary schema, or zero if
it has none (because it has not created any temporary tables).
diff --git a/src/backend/port/sysv_shmem.c b/src/backend/port/sysv_shmem.c
index eaba244bc9c..9ff2d7b3d38 100644
--- a/src/backend/port/sysv_shmem.c
+++ b/src/backend/port/sysv_shmem.c
@@ -619,22 +619,24 @@ 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
+ huge_pages_active = (ptr != MAP_FAILED);
+
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..f97971ef340 100644
--- a/src/backend/port/win32_shmem.c
+++ b/src/backend/port/win32_shmem.c
@@ -319,22 +319,24 @@ 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;
}
+
+ huge_pages_active = ((flProtect & SEC_LARGE_PAGES) != 0);
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/adt/misc.c b/src/backend/utils/adt/misc.c
index 5d78d6dc060..3c175d88c59 100644
--- a/src/backend/utils/adt/misc.c
+++ b/src/backend/utils/adt/misc.c
@@ -1070,11 +1070,22 @@ pg_get_replica_identity_index(PG_FUNCTION_ARGS)
PG_RETURN_NULL();
}
/*
* Transition function for the ANY_VALUE aggregate
*/
Datum
any_value_transfn(PG_FUNCTION_ARGS)
{
PG_RETURN_DATUM(PG_GETARG_DATUM(0));
}
+
+bool huge_pages_active = false;
+
+/*
+ * Return status of huge pages.
+ */
+Datum
+pg_huge_pages_active(PG_FUNCTION_ARGS)
+{
+ PG_RETURN_BOOL(huge_pages_active);
+}
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index ae658e721fd..7cfa11c994c 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -1811,22 +1811,27 @@
proname => 'bpchar', prorettype => 'bpchar', proargtypes => 'char',
prosrc => 'char_bpchar' },
{ oid => '861', descr => 'name of the current database',
proname => 'current_database', provolatile => 's', prorettype => 'name',
proargtypes => '', prosrc => 'current_database' },
{ oid => '817', descr => 'get the currently executing query',
proname => 'current_query', proisstrict => 'f', provolatile => 'v',
proparallel => 'r', prorettype => 'text', proargtypes => '',
prosrc => 'current_query' },
+{ oid => '8218', descr => 'get the state of huge pages',
+ proname => 'pg_huge_pages_active', proisstrict => 't', provolatile => 's',
+ proparallel => 's', prorettype => 'bool', proargtypes => '',
+ prosrc => 'pg_huge_pages_active' },
+
{ oid => '3399',
proname => 'int8_mul_cash', prorettype => 'money',
proargtypes => 'int8 money', prosrc => 'int8_mul_cash' },
{ oid => '862',
proname => 'int4_mul_cash', prorettype => 'money',
proargtypes => 'int4 money', prosrc => 'int4_mul_cash' },
{ oid => '863',
proname => 'int2_mul_cash', prorettype => 'money',
proargtypes => 'int2 money', prosrc => 'int2_mul_cash' },
{ oid => '3344',
proname => 'cash_mul_int8', prorettype => 'money',
diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h
index 06a86f9ac1f..8169e5bd6dd 100644
--- a/src/include/miscadmin.h
+++ b/src/include/miscadmin.h
@@ -268,22 +268,25 @@ extern PGDLLIMPORT int VacuumCostPageMiss;
extern PGDLLIMPORT int VacuumCostPageDirty;
extern PGDLLIMPORT int VacuumCostLimit;
extern PGDLLIMPORT double VacuumCostDelay;
extern PGDLLIMPORT int64 VacuumPageHit;
extern PGDLLIMPORT int64 VacuumPageMiss;
extern PGDLLIMPORT int64 VacuumPageDirty;
extern PGDLLIMPORT int VacuumCostBalance;
extern PGDLLIMPORT bool VacuumCostActive;
+/* in adt/misc.c */
+extern PGDLLIMPORT bool huge_pages_active;
+
/* in tcop/postgres.c */
typedef char *pg_stack_base_t;
extern pg_stack_base_t set_stack_base(void);
extern void restore_stack_base(pg_stack_base_t base);
extern void check_stack_depth(void);
extern bool stack_is_too_deep(void);
/* in tcop/utility.c */
--
2.34.1
view thread (17+ messages) latest in thread
reply
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: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: Improve logging when using Huge Pages
In-Reply-To: <ZA+Bpk/[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox