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....@gmail.com] 
Sent: Wednesday, September 8, 2021 11:18 AM
To: pry...@telsasoft.com
Cc: masao.fu...@oss.nttdata.com; Shinoda, Noriyoshi (PN Japan FSIP) 
<noriyoshi.shin...@hpe.com>; pgsql-hack...@postgresql.org; rjuju...@gmail.com; 
t...@sss.pgh.pa.us
Subject: Re: Improve logging when using Huge Pages

At Tue, 7 Sep 2021 08:16:53 -0500, Justin Pryzby <pry...@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

Attachment: huge_pages_log_v4.diff
Description: huge_pages_log_v4.diff

Reply via email to