Re: Improve logging when using Huge Pages

Justin Pryzby <pryzby@telsasoft.com>

From: Justin Pryzby <pryzby@telsasoft.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: Kyotaro Horiguchi <horikyota.ntt@gmail.com>, sfrost@snowman.net, alvherre@alvh.no-ip.org, andres@anarazel.de, nathandbossart@gmail.com, 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-03-24T01:50:50Z
Lists: pgsql-hackers

Attachments

On Thu, Mar 23, 2023 at 05:25:46PM +0900, Kyotaro Horiguchi wrote:
> Wouldn't storing the value in the shared memory itself work? Though,
> that space does become almost dead for the server's lifetime...

I'm sure it's possible, but it's also not worth writing a special
implementation just to handle huge_pages_active, which is better written
in 30 lines than in 300 lines.

If we needed to avoid using a GUC, maybe it'd work to add
huge_pages_active to PGShmemHeader.  But "avoid using gucs at all costs"
isn't the goal, and using a GUC parallels all the similar, and related
things without needing to allocate extra bits of shared_memory.

On Thu, Mar 23, 2023 at 07:23:28AM +0900, Michael Paquier wrote:
> On Wed, Mar 22, 2023 at 05:18:28PM -0500, Justin Pryzby wrote:
> > Wow, good point.  I think to make it work we'd need put
> > huge_pages_active into BackendParameters and handle it in
> > save_backend_variables().  If so, that'd be strong argument for using a
> > GUC, which already has all the necessary infrastructure for exposing the
> > server's state.
> 
> I am afraid so, duplicating an existing infrastructure for a need like
> the one of this thread is not really appealing.

This goes back to using a GUC, and:

 - removes GUC_RUNTIME_COMPUTED, since that causes a useless error when
   using -C if the server is running, rather than successfully printing
   "unknown".
 - I also renamed it from huge_pages_active = {true,false,unknown} to
   huge_pages_STATUS = {on,off,unknown}.  This parallels huge_pages,
   which is documented to accept values on/off/try.  Also, true/false
   isn't how bools are displayed.
 - I realized that the rename suggested implementing it as an enum GUC,
   and re-using the existing HUGE_PAGES_{ON,OFF} values (and adding an
   "UNKNOWN").  Maybe this also avoids Stephen's earlier objection to
   using a string ?

I mis-used cirrusci to check that the GUC does work correctly under
windows.

If there's continuing aversions to using a GUC, please say so, and try
to suggest an alternate implementation you think would be justified.
It'd need to work under windows with EXEC_BACKEND.

-- 
Justin

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