Re: Improve logging when using Huge Pages - Mailing list pgsql-hackers

From Kyotaro Horiguchi
Subject Re: Improve logging when using Huge Pages
Date
Msg-id 20210903.164934.1726669051525427478.horikyota.ntt@gmail.com
Whole thread Raw
In response to RE: Improve logging when using Huge Pages  ("Shinoda, Noriyoshi (PN Japan FSIP)" <noriyoshi.shinoda@hpe.com>)
Responses Re: Improve logging when using Huge Pages
List pgsql-hackers
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
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.
 

IF you are thinking to show that in GUC, you might want to look into
the nearby thread [1], 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] https://www.postgresql.org/message-id/20210903.141206.103927759882272221.horikyota.ntt%40gmail.com



pgsql-hackers by date:

Previous
From: Kyotaro Horiguchi
Date:
Subject: Re: prevent immature WAL streaming
Next
From: Nitin Jadhav
Date:
Subject: Re: when the startup process doesn't (logging startup delays)