Subject: Re: Improve logging when using Huge Pages
To: Julien Rouhaud <rjuju123@gmail.com>,
 "Shinoda, Noriyoshi (PN Japan FSIP)" <noriyoshi.shinoda@hpe.com>
Cc: "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
References: 
 <TU4PR8401MB1152EBB0D271F827E2E37A01EECC9@TU4PR8401MB1152.NAMPRD84.PROD.OUTLOOK.COM>
 <CAOBaU_YGfFyruW4YX6++DifTtBt8bjatz0gMhaTRD=dqV3gdWg@mail.gmail.com>
From: Fujii Masao <masao.fujii@oss.nttdata.com>
Message-ID: <7e5b9f6d-1db9-d1e2-3bd8-ef65f24b35be@oss.nttdata.com>
Date: Wed, 1 Sep 2021 02:05:53 +0900
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:78.0)
 Gecko/20100101 Thunderbird/78.13.0
MIME-Version: 1.0
In-Reply-To: 
 <CAOBaU_YGfFyruW4YX6++DifTtBt8bjatz0gMhaTRD=dqV3gdWg@mail.gmail.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Archived-At: 
 <https://www.postgresql.org/message-id/7e5b9f6d-1db9-d1e2-3bd8-ef65f24b35be%40oss.nttdata.com>
Precedence: bulk


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 success or failure of the HugePages acquisition. If you want to output logs, you need to set log_min_messages=DEBUG3, but it 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 notice that 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 that administrators 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