Date: Wed, 8 Mar 2023 14:16:56 -0800
From: Nathan Bossart <nathandbossart@gmail.com>
To: Justin Pryzby <pryzby@telsasoft.com>
Cc: Alvaro Herrera <alvherre@alvh.no-ip.org>,
	Andres Freund <andres@anarazel.de>,
	Michael Paquier <michael@paquier.xyz>,
	Kyotaro Horiguchi <horikyota.ntt@gmail.com>, thomas.munro@gmail.com,
	tgl@sss.pgh.pa.us, john.naylor@enterprisedb.com,
	noriyoshi.shinoda@hpe.com, jchampion@timescale.com,
	sawada.mshk@gmail.com, masao.fujii@oss.nttdata.com,
	pgsql-hackers@postgresql.org, rjuju123@gmail.com
Subject: Re: Improve logging when using Huge Pages
Message-ID: <20230308221656.GA3621885@nathanxps13>
References: <20230208231803.GP1653@telsasoft.com>
 <20230209094013.67varf63bihops33@alvherre.pgsql>
 <20230209192909.GA606606@nathanxps13>
 <20230213232245.GI1653@telsasoft.com>
 <20230214041852.GA1190218@nathanxps13>
 <20230215013256.GP1653@telsasoft.com>
 <20230215181317.GA1353037@nathanxps13>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230215181317.GA1353037@nathanxps13>
Archived-At: 
 <https://www.postgresql.org/message-id/20230308221656.GA3621885%40nathanxps13>
Precedence: bulk

On Wed, Feb 15, 2023 at 10:13:17AM -0800, Nathan Bossart wrote:
> On Tue, Feb 14, 2023 at 07:32:56PM -0600, Justin Pryzby wrote:
>> On Mon, Feb 13, 2023 at 08:18:52PM -0800, Nathan Bossart wrote:
>>> I'm curious why you chose to make this a string instead of an enum.  There
>>> might be little practical difference, but since there are only three
>>> possible values, I wonder if it'd be better form to make it an enum.
>> 
>> It takes more code to write as an enum - see 002.txt.  I'm not convinced
>> this is better.
>> 
>> But your comment made me fix its <type>, and reconsider the strings,
>> which I changed to active={unknown/true/false} rather than {unk/on/off}.
>> It could also be active={unknown/yes/no}...
> 
> I think unknown/true/false is fine.  I'm okay with using a string if no one
> else thinks it should be an enum (or a bool).

There's been no response for this, so I guess we can proceed with a string
GUC.

+        Reports whether huge pages are in use by the current instance.
+        See <xref linkend="guc-huge-pages"/> for more information.

I still think we should say "server" in place of "current instance" here.

+		{"huge_pages_active", PGC_INTERNAL, PRESET_OPTIONS,
+			gettext_noop("Indicates whether huge pages are in use."),
+			NULL,
+			GUC_NOT_IN_SAMPLE | GUC_DISALLOW_IN_FILE | GUC_RUNTIME_COMPUTED
+		},

I don't think we need to use GUC_RUNTIME_COMPUTED.  'postgres -C' seems to
always report "unknown" for this GUC, so all this would do is cause that
command to error unnecessarily when the server is running.

It might be worth documenting exactly what "unknown" means.  IIUC you'll
only ever see "on" or "off" via SHOW or pg_settings, which doesn't seem
tremendously obvious.

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com