RE: Improve logging when using Huge Pages

Fujii-san, Julien-san

Thank you very much for your comment.
I followed your comment and changed the elog function to ereport function and also changed the log level. The output
messageis the same as in the case of non-HugePages memory acquisition failure.I did not simplify the error messages as
itwould have complicated the response to the preprocessor.
 

> I agree that the message should be promoted to a higher level.  But I 
> think we should also make that information available at the SQL level, 
> as the log files may be truncated / rotated before you need the info, 
> and it can be troublesome to find the information at the OS level, if 
> you're lucky enough to have OS access.

In the attached patch, I have added an Internal GUC 'using_huge_pages' to know that it is using HugePages. This
parameterwill be True only if the instance is using HugePages.
 

Regards,
Noriyoshi Shinoda

-----Original Message-----
From: Fujii Masao [mailto:masao.fujii@oss.nttdata.com] 
Sent: Wednesday, September 1, 2021 2:06 AM
To: Julien Rouhaud <rjuju123@gmail.com>; Shinoda, Noriyoshi (PN Japan FSIP) <noriyoshi.shinoda@hpe.com>
Cc: pgsql-hackers@postgresql.org
Subject: Re: Improve logging when using Huge Pages



On 2021/08/31 15:57, Julien Rouhaud wrote:
> On Tue, Aug 31, 2021 at 1:37 PM Shinoda, Noriyoshi (PN Japan FSIP) 
> <noriyoshi.shinoda@hpe.com> wrote:
>>
>> In the current version, when GUC huge_pages=try, which is the default setting, no log is output regardless of the
successor failure of the HugePages acquisition. If you want to output logs, you need to set log_min_messages=DEBUG3,
butit will output a huge amount of extra logs.
 
>> With huge_pages=try setting, if the kernel parameter vm.nr_hugepages is not enough, the administrator will not
noticethat HugePages is not being used.
 
>> I think it should output a log if HugePages was not available.

+1

-            elog(DEBUG1, "mmap(%zu) with MAP_HUGETLB failed, huge pages disabled: %m",
+            elog(WARNING, "mmap(%zu) with MAP_HUGETLB failed, huge pages 
+disabled: %m",

elog() should be used only for internal errors and low-level debug logging.
So per your proposal, elog() is not suitable here. Instead, ereport() should be used.

The log level should be LOG rather than WARNING because this message indicates the information about server activity
thatadministrators are interested in.
 

The message should be updated so that it follows the Error Message Style Guide.
https://www.postgresql.org/docs/devel/error-style-guide.html 

With huge_pages=on, if shared memory fails to be allocated, the error message is reported currently. Even with
huge_page=try,this error message should be used to simplify the code as follows?
 

                 errno = mmap_errno;
-               ereport(FATAL,
+               ereport((huge_pages == HUGE_PAGES_ON) ? FATAL : LOG,
                                 (errmsg("could not map anonymous shared memory: %m"),
                                  (mmap_errno == ENOMEM) ?
                                  errhint("This error usually means that PostgreSQL's request "



> I agree that the message should be promoted to a higher level.  But I 
> think we should also make that information available at the SQL level, 
> as the log files may be truncated / rotated before you need the info, 
> and it can be troublesome to find the information at the OS level, if 
> you're lucky enough to have OS access.

+1

Regards,

--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION