Improve logging when using Huge Pages
Hi Hackers,
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.
By the way, in MySQL with almost the same architecture, the following log is output at the Warning level.
[Warning] [MY-012677] [InnoDB] Failed to allocate 138412032 bytes. errno 1
[Warning] [MY-012679] [InnoDB] Using conventional memory pool
The attached small patch outputs a log at the WARNING level when huge_pages = try and if the acquisition of HugePages fails.
Regards,
Noriyoshi Shinoda
Attachments:
huge_pages_log_v1.diffapplication/octet-stream; name=huge_pages_log_v1.diffDownload+2-2
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.
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.
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
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 message is the same as in the case of non-HugePages memory acquisition failure.I did not simplify the error messages as it would 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 parameter will 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 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
Attachments:
huge_pages_log_v2.diffapplication/octet-stream; name=huge_pages_log_v2.diffDownload+31-6
Hello.
At Fri, 3 Sep 2021 06:28:58 +0000, "Shinoda, Noriyoshi (PN Japan FSIP)" <noriyoshi.shinoda@hpe.com> wrote in
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 message is the same as in the case of non-HugePages memory acquisition failure.I did not simplify the error messages as it would 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 parameter will be True only if the instance is using HugePages.
IF you are thinking to show that in GUC, you might want to look into
the nearby thread [1]/messages/by-id/20210903.141206.103927759882272221.horikyota.ntt@gmail.com, especially about the behavior when invoking
postgres -C using_huge_pages. (Even though the word "using" in the
name may suggest that the server is running, but I don't think it is
neat that the variable shows "no" by the command but shows "yes" while
the same server is running.)
I have some comment about the patch.
- if (huge_pages == HUGE_PAGES_TRY && ptr == MAP_FAILED)
- elog(DEBUG1, "mmap(%zu) with MAP_HUGETLB failed, huge pages disabled: %m",
- allocsize);
+ if (ptr != MAP_FAILED)
+ using_huge_pages = true;
+ else if (huge_pages == HUGE_PAGES_TRY)
+ ereport(LOG,
+ (errmsg("could not map anonymous shared memory: %m"),
+ (mmap_errno == ENOMEM) ?
+ errhint("This error usually means that PostgreSQL's request "If we set huge_pages to try and postgres falled back to regular pages,
it emits a large message relative to its importance. The user specifed
that "I'd like to use huge pages, but it's ok if not available.", so I
think the message should be far smaller. Maybe just raising the
DEBUG1 message to LOG along with moving to ereport might be
sufficient.
- elog(DEBUG1, "CreateFileMapping(%zu) with SEC_LARGE_PAGES failed, "
- "huge pages disabled",
- size);
+ ereport(LOG,
+ (errmsg("could not create shared memory segment: error code %lu", GetLastError()),
+ errdetail("Failed system call was CreateFileMapping(size=%zu, name=%s).",
+ size, szShareMem)));It doesn't seem to be a regular user-facing message. Isn't it
sufficient just to raise the log level to LOG?
[1]: /messages/by-id/20210903.141206.103927759882272221.horikyota.ntt@gmail.com
On 2021/09/03 16:49, Kyotaro Horiguchi wrote:
IF you are thinking to show that in GUC, you might want to look into
the nearby thread [1]
Yes, let's discuss this feature at that thread.
I have some comment about the patch.
- if (huge_pages == HUGE_PAGES_TRY && ptr == MAP_FAILED) - elog(DEBUG1, "mmap(%zu) with MAP_HUGETLB failed, huge pages disabled: %m", - allocsize); + if (ptr != MAP_FAILED) + using_huge_pages = true; + else if (huge_pages == HUGE_PAGES_TRY) + ereport(LOG, + (errmsg("could not map anonymous shared memory: %m"), + (mmap_errno == ENOMEM) ? + errhint("This error usually means that PostgreSQL's request "If we set huge_pages to try and postgres falled back to regular pages,
it emits a large message relative to its importance. The user specifed
that "I'd like to use huge pages, but it's ok if not available.", so I
think the message should be far smaller. Maybe just raising the
DEBUG1 message to LOG along with moving to ereport might be
sufficient.
IMO, if the level is promoted to LOG, the message should be updated
so that it follows the error message style guide. But I agree that simpler
message would be better in this case. So what about something like
the following?
LOG: could not map anonymous shared memory (%zu bytes) with huge pages enabled
HINT: The server will map anonymous shared memory again with huge pages disabled.
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
Fujii Masao <masao.fujii@oss.nttdata.com> writes:
IMO, if the level is promoted to LOG, the message should be updated
so that it follows the error message style guide. But I agree that simpler
message would be better in this case. So what about something like
the following?
LOG: could not map anonymous shared memory (%zu bytes) with huge pages enabled
HINT: The server will map anonymous shared memory again with huge pages disabled.
That is not a hint. Maybe it qualifies as errdetail, though.
regards, tom lane
On 2021/09/03 23:27, Tom Lane wrote:
Fujii Masao <masao.fujii@oss.nttdata.com> writes:
IMO, if the level is promoted to LOG, the message should be updated
so that it follows the error message style guide. But I agree that simpler
message would be better in this case. So what about something like
the following?LOG: could not map anonymous shared memory (%zu bytes) with huge pages enabled
HINT: The server will map anonymous shared memory again with huge pages disabled.That is not a hint. Maybe it qualifies as errdetail, though.
Yes, agreed.
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
Hello,
Thank you everyone for comments.
In the thread [1]/messages/by-id/20210903.141206.103927759882272221.hor that Horiguchi told me about, there is already a review going on about GUC for HugePages memory.
For this reason, I have removed the new GUC implementation and attached a patch that changes only the message at instance startup.
[1]: /messages/by-id/20210903.141206.103927759882272221.hor
/messages/by-id/20210903.141206.103927759882272221.hor
Regards,
Noriyoshi Shinoda
-----Original Message-----
From: Fujii Masao [mailto:masao.fujii@oss.nttdata.com]
Sent: Saturday, September 4, 2021 1:36 AM
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Kyotaro Horiguchi <horikyota.ntt@gmail.com>; Shinoda, Noriyoshi (PN Japan FSIP) <noriyoshi.shinoda@hpe.com>; rjuju123@gmail.com; pgsql-hackers@postgresql.org
Subject: Re: Improve logging when using Huge Pages
On 2021/09/03 23:27, Tom Lane wrote:
Fujii Masao <masao.fujii@oss.nttdata.com> writes:
IMO, if the level is promoted to LOG, the message should be updated
so that it follows the error message style guide. But I agree that
simpler message would be better in this case. So what about something
like the following?LOG: could not map anonymous shared memory (%zu bytes) with huge
pages enabled
HINT: The server will map anonymous shared memory again with huge pages disabled.That is not a hint. Maybe it qualifies as errdetail, though.
Yes, agreed.
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
Attachments:
huge_pages_log_v3.diffapplication/octet-stream; name=huge_pages_log_v3.diffDownload+10-5
On 2021/09/07 13:09, Shinoda, Noriyoshi (PN Japan FSIP) wrote:
Hello,
Thank you everyone for comments.
In the thread [1] that Horiguchi told me about, there is already a review going on about GUC for HugePages memory.
For this reason, I have removed the new GUC implementation and attached a patch that changes only the message at instance startup.
Thanks for updating the patch!
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.
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.
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
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.")));--
Justin
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
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:
huge_pages_log_v4.diffapplication/octet-stream; name=huge_pages_log_v4.diffDownload+6-5
Thanks!
At Wed, 8 Sep 2021 07:52:35 +0000, "Shinoda, Noriyoshi (PN Japan FSIP)" <noriyoshi.shinoda@hpe.com> wrote in
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.
Ah, right.
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.
It seems *to me* sufficient. I'm not sure what cases CreateFileMapping
return ERROR_NO_SYSTEM_RESOURCES when non-huge page can be allocated
successfully, though, but that doesn't matter much, maybe.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
On 2021/09/07 22:16, Justin Pryzby wrote:
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.
I was afraid that logging the message like "could not ..." every time
when the server starts up would surprise users unnecessarily.
Because the message sounds like it reports a server error.
So it might be good idea to change the message to something like
"disabling huge pages" to avoid such surprise.
If it's a problem, the
message could be a NOTICE or INFO, and it won't be shown by default.
That's an idea, but neither NOTICE nor INFO are suitable for
this kind of message....
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
On 2021/09/08 11:17, Kyotaro Horiguchi wrote:
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"
How about simpler message like "disabling huge pages" or
"disabling huge pages due to lack of huge pages available"?
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
At Fri, 17 Sep 2021 00:13:41 +0900, Fujii Masao <masao.fujii@oss.nttdata.com> wrote in
On 2021/09/08 11:17, Kyotaro Horiguchi wrote:
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"How about simpler message like "disabling huge pages" or
"disabling huge pages due to lack of huge pages available"?
Honestly, I cannot have conficence on my wording, but "disabling huge
pages" souds like somthing that happens on the OS layer. "did not
use/gave up using huge pages for anounymous shared memory" might work
well, I'm not sure, though...
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
Hi,
Thank you for your comment.
I was afraid that logging the message like "could not ..." every time when the server starts up would surprise users unnecessarily.
Because the message sounds like it reports a server error.
Fujii-san,
I was worried about the same thing as you.
So the attached patch gets the value of the kernel parameter vm.nr_hugepages,
and if it is the default value of zero, the log level is the same as before.
On the other hand, if any value is set, the level is set to LOG.
I hope I can find a better message other than this kind of implementation.
Regards,
Noriyoshi Shinoda
-----Original Message-----
From: Kyotaro Horiguchi [mailto:horikyota.ntt@gmail.com]
Sent: Friday, September 17, 2021 1:15 PM
To: masao.fujii@oss.nttdata.com
Cc: pryzby@telsasoft.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 Fri, 17 Sep 2021 00:13:41 +0900, Fujii Masao <masao.fujii@oss.nttdata.com> wrote in
On 2021/09/08 11:17, Kyotaro Horiguchi wrote:
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"How about simpler message like "disabling huge pages" or "disabling
huge pages due to lack of huge pages available"?
Honestly, I cannot have conficence on my wording, but "disabling huge pages" souds like somthing that happens on the OS layer. "did not use/gave up using huge pages for anounymous shared memory" might work well, I'm not sure, though...
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
Attachments:
huge_pages_log_v5.diffapplication/octet-stream; name=huge_pages_log_v5.diffDownload+15-8
On 2021/09/20 17:55, Shinoda, Noriyoshi (PN Japan FSIP) wrote:
I was worried about the same thing as you.
So the attached patch gets the value of the kernel parameter vm.nr_hugepages,
and if it is the default value of zero, the log level is the same as before.
On the other hand, if any value is set, the level is set to LOG.
Probably I understood your point. But isn't it more confusing to users?
Because whether the log message is output or not is changed depending on
the setting of the kernel parameter. For example, when vm.nr_hugepages=0
and no log message about huge pages is output, users might easily misunderstand
that shared memory was successfully allocated with huge pages because
they saw no such log message.
IMO we should leave the log message "mmap(%zu) with MAP_HUGETLB failed..."
as it is if users don't like additional log message output whenever
the server restarts. In this case, instead, maybe it's better to provide GUC or
something to report whether shared memory was successfully allocated
with huge pages or not.
OTOH, if users can accept such additional log message, I think that it's
less confusing to report something like "disable huge pages ..." whenever
mmap() with huge pages fails. I still prefer "disable huge pages ..." to
"fall back ..." as the log message, but if many people think the latter is
better, I'd follow that.
Another idea is to output "Anonymous shared memory was allocated with
huge pages" when it's successfully allocated with huge pages, and to output
"Anonymous shared memory was allocated without huge pages"
when it's successfully allocated without huge pages. I'm not sure if users
may think even this message is noisy, though.
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
On Wed, Sep 22, 2021 at 02:03:11AM +0900, Fujii Masao wrote:
Another idea is to output "Anonymous shared memory was allocated with
huge pages" when it's successfully allocated with huge pages, and to output
"Anonymous shared memory was allocated without huge pages"
when it's successfully allocated without huge pages. I'm not sure if users
may think even this message is noisy, though.
+1
Maybe it could show the page size instead of "with"/without:
"Shared memory allocated with 4k/1MB/1GB pages."
--
Justin
