RE: Improve logging when using Huge Pages

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