From: Shinoda, Noriyoshi (PN Japan FSIP) <noriyoshi.shinoda@hpe.com>
To: masao.fujii@oss.nttdata.com <masao.fujii@oss.nttdata.com>
Cc: pgsql-hackers@postgresql.org <pgsql-hackers@postgresql.org>
Cc: rjuju123@gmail.com <rjuju123@gmail.com>
Cc: tgl@sss.pgh.pa.us <tgl@sss.pgh.pa.us>
Cc: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Cc: pryzby@telsasoft.com <pryzby@telsasoft.com>
Subject: RE: Improve logging when using Huge Pages
Date: Wed, 8 Sep 2021 07:52:35 +0000
Message-ID: <DF4PR8401MB1147ADD49D7236F6FBDCABF5EED49@DF4PR8401MB1147.NAMPRD84.PROD.OUTLOOK.COM> (raw)
In-Reply-To: <20210908.111736.355857044349579789.horikyota.ntt@gmail.com>
References: <TU4PR8401MB115257079749D8958A3450F7EED39@TU4PR8401MB1152.NAMPRD84.PROD.OUTLOOK.COM>
<b707355a-84ba-9f8b-6026-8fac36121279@oss.nttdata.com>
<20210907131653.GK26465@telsasoft.com>
<20210908.111736.355857044349579789.horikyota.ntt@gmail.com>
Hello,
Thank you everyone for comments.
I have attached a patch that simply changed the message like the advice from Horiguchi-san.
> Even with the patch, there are still some cases where huge pages is disabled silently. We should report something even in these cases?
> For example, in the platform where huge pages is not supported, it's silently disabled when huge_pages=try.
The area where this patch is written is inside the "#ifdef MAP_HUGETLB #endif" block.
For this reason, I think it is excluded from binaries created in an environment that does not have the MAP_HUGETLB macro.
> One big concern about the patch is that log message is always reported when shared memory fails to be allocated with huge pages enabled when huge_pages=try. Since
> huge_pages=try is the default setting, many users would see this new log message whenever they start the server. Those who don't need huge pages but just use the default
> setting might think that such log messages would be noisy.
This patch is meant to let the admin know that HugePages isn't being used, so I'm sure you're right. I have no idea what to do so far.
Regards,
Noriyoshi Shinoda
-----Original Message-----
From: Kyotaro Horiguchi [mailto:horikyota.ntt@gmail.com]
Sent: Wednesday, September 8, 2021 11:18 AM
To: pryzby@telsasoft.com
Cc: masao.fujii@oss.nttdata.com; Shinoda, Noriyoshi (PN Japan FSIP) <noriyoshi.shinoda@hpe.com>; pgsql-hackers@postgresql.org; rjuju123@gmail.com; tgl@sss.pgh.pa.us
Subject: Re: Improve logging when using Huge Pages
At Tue, 7 Sep 2021 08:16:53 -0500, Justin Pryzby <pryzby@telsasoft.com> wrote in
> On Tue, Sep 07, 2021 at 07:12:36PM +0900, Fujii Masao wrote:
> > One big concern about the patch is that log message is always
> > reported when shared memory fails to be allocated with huge pages
> > enabled when huge_pages=try. Since huge_pages=try is the default
> > setting, many users would see this new log message whenever they
> > start the server. Those who don't need huge pages but just use the
> > default setting might think that such log messages would be noisy.
>
> I don't see this as any issue. We're only talking about a single
> message on each restart, which would be added in a major release. If
> it's a problem, the message could be a NOTICE or INFO, and it won't be shown by default.
>
> I think it should say "with/out huge pages" without
> "enabled/disabled", without "again", and without "The server", like:
>
> + (errmsg("could not map anonymous shared memory (%zu bytes)"
> + " with huge pages.", allocsize),
> + errdetail("Anonymous shared memory will be mapped "
> + "without huge
> + pages.")));
I don't dislike the message, but I'm not sure I like the message is too verbose, especially about it has DETAILS. It seems to me something like the following is sufficient, or I'd like see it even more concise.
"fall back anonymous shared memory to non-huge pages: required %zu bytes for huge pages"
If we emit an error message for other than mmap failure, it would be like the following.
"fall back anonymous shared memory to non-huge pages: huge pages not available"
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
Attachments:
[application/octet-stream] huge_pages_log_v4.diff (1.2K, ../DF4PR8401MB1147ADD49D7236F6FBDCABF5EED49@DF4PR8401MB1147.NAMPRD84.PROD.OUTLOOK.COM/2-huge_pages_log_v4.diff)
download | inline diff:diff --git a/src/backend/port/sysv_shmem.c b/src/backend/port/sysv_shmem.cindex 9de96ed..cd98007 100644--- a/src/backend/port/sysv_shmem.c+++ b/src/backend/port/sysv_shmem.c@@ -592,8 +592,9 @@ CreateAnonymousSegment(Size *size)
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);+ ereport(LOG,+ (errmsg("fall back anonymous shared memory to non-huge "+ "pages: required %zu bytes for huge pages", allocsize)));
}
#endif
diff --git a/src/backend/port/win32_shmem.c b/src/backend/port/win32_shmem.cindex d7a7199..9a9e98a 100644--- a/src/backend/port/win32_shmem.c+++ b/src/backend/port/win32_shmem.c@@ -293,9 +293,9 @@ retry:
huge_pages == HUGE_PAGES_TRY &&
(flProtect & SEC_LARGE_PAGES) != 0)
{
- elog(DEBUG1, "CreateFileMapping(%zu) with SEC_LARGE_PAGES failed, "- "huge pages disabled",- size);+ ereport(LOG,+ (errmsg("fall back anonymous shared memory to non-huge "+ "pages: required %zu bytes for huge pages", size)));
/*
* Use the original size, not the rounded-up value, when
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: noriyoshi.shinoda@hpe.com, masao.fujii@oss.nttdata.com, rjuju123@gmail.com, tgl@sss.pgh.pa.us, horikyota.ntt@gmail.com, pryzby@telsasoft.com
Subject: RE: Improve logging when using Huge Pages
In-Reply-To: <DF4PR8401MB1147ADD49D7236F6FBDCABF5EED49@DF4PR8401MB1147.NAMPRD84.PROD.OUTLOOK.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