Re: Improve logging when using Huge Pages

Michael Paquier <michael@paquier.xyz>

From: Michael Paquier <michael@paquier.xyz>
To: Nathan Bossart <nathandbossart@gmail.com>
Cc: Justin Pryzby <pryzby@telsasoft.com>, Kyotaro Horiguchi <horikyota.ntt@gmail.com>, sfrost@snowman.net, alvherre@alvh.no-ip.org, andres@anarazel.de, jchampion@timescale.com, john.naylor@enterprisedb.com, masao.fujii@oss.nttdata.com, noriyoshi.shinoda@hpe.com, pgsql-hackers@postgresql.org, rjuju123@gmail.com, sawada.mshk@gmail.com, tgl@sss.pgh.pa.us, thomas.munro@gmail.com
Date: 2023-06-13T05:50:30Z
Lists: pgsql-hackers
On Mon, Jun 12, 2023 at 02:37:15PM -0700, Nathan Bossart wrote:
> Fair enough.  I know I've been waffling in the GUC versus function
> discussion, but FWIW v7 of the patch looks reasonable to me.

+       Assert(strcmp("unknown", GetConfigOption("huge_pages_status", false, false)) != 0);

Not sure that there is anything to gain with this assertion in
CreateSharedMemoryAndSemaphores(), because this is pretty much what
check_GUC_init() looks after?

@@ -627,6 +627,9 @@ CreateAnonymousSegment(Size *size)
    }
 #endif
 
+   SetConfigOption("huge_pages_status", ptr == MAP_FAILED ? "off" : "on",
+                   PGC_INTERNAL, PGC_S_DYNAMIC_DEFAULT);

The choice of this location is critical, because this is just *after*
we've tried huge pages, but *before* doing the fallback mmap() call
when the huge page allocation was on "try".  I think that this
deserves a comment.

@@ -327,6 +327,10 @@ retry:
            Sleep(1000);
            continue;
        }
+
+       SetConfigOption("huge_pages_status", (flProtect & SEC_LARGE_PAGES) ?
+                       "on" : "off", PGC_INTERNAL, PGC_S_DYNAMIC_DEFAULT);

Hmm.  I still think that it is cleaner to move that at the end of
PGSharedMemoryCreate() for the WIN32 case.  There are also few FATALs
in-between, which would make SetConfigOption() useless if there is an
in-flight problem.

Don't we need to update save_backend_variables() and add an entry
in BackendParameters to make other processes launched by EXEC_BACKEND
inherit the status value set?
--
Michael

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →
  1. Add GUC parameter "huge_pages_status"

  2. Add check on initial and boot values when loading GUCs

  3. Clean up some inconsistencies with GUC declarations

  4. Clean up some GUC declarations and comments