Received: from malur.postgresql.org ([217.196.149.56])
	by arkaria.postgresql.org with esmtps
 (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256)
	(Exim 4.92)
	(envelope-from <pgsql-hackers-owner+archive@lists.postgresql.org>)
	id 1paMTC-0000sA-EF
	for pgsql-hackers@arkaria.postgresql.org; Thu, 09 Mar 2023 20:02:34 +0000
Received: from localhost ([127.0.0.1] helo=malur.postgresql.org)
	by malur.postgresql.org with esmtp (Exim 4.92)
	(envelope-from <pgsql-hackers-owner+archive@lists.postgresql.org>)
	id 1paMTB-0007uv-28
	for pgsql-hackers@arkaria.postgresql.org; Thu, 09 Mar 2023 20:02:33 +0000
Received: from makus.postgresql.org ([2001:4800:3e1:1::229])
	by malur.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256)
	(Exim 4.92)
	(envelope-from <sfrost@tamriel.snowman.net>)
	id 1paMTA-0007um-K5
	for pgsql-hackers@lists.postgresql.org; Thu, 09 Mar 2023 20:02:32 +0000
Received: from tamriel.snowman.net ([70.109.60.50])
	by makus.postgresql.org with esmtp (Exim 4.92)
	(envelope-from <sfrost@tamriel.snowman.net>)
	id 1paMT8-0000tl-27
	for pgsql-hackers@postgresql.org; Thu, 09 Mar 2023 20:02:31 +0000
Received: by tamriel.snowman.net (Postfix, from userid 1000)
	id 73FA85F7BC; Thu,  9 Mar 2023 15:02:29 -0500 (EST)
Date: Thu, 9 Mar 2023 15:02:29 -0500
From: Stephen Frost <sfrost@snowman.net>
To: Justin Pryzby <pryzby@telsasoft.com>
Cc: Nathan Bossart <nathandbossart@gmail.com>,
	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: <ZAo7VQCm1UBt9spR@tamriel.snowman.net>
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>
 <20230308221656.GA3621885@nathanxps13>
 <ZAnuYi/Cb8QjRSoN@tamriel.snowman.net>
 <ZAoLoCPRf6j/HyQp@telsasoft.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="4GcfYaGCt2KsK7LC"
Content-Disposition: inline
In-Reply-To: <ZAoLoCPRf6j/HyQp@telsasoft.com>
User-Agent: Mutt/2.1.4 (2021-12-11)
List-Id: <pgsql-hackers.lists.postgresql.org>
List-Help: <https://lists.postgresql.org/manage/>
List-Subscribe: <https://lists.postgresql.org/manage/>
List-Post: <mailto:pgsql-hackers@lists.postgresql.org>
List-Owner: <mailto:pgsql-hackers-owner@lists.postgresql.org>
List-Archive: <https://www.postgresql.org/list/pgsql-hackers>
Archived-At: 
 <https://www.postgresql.org/message-id/ZAo7VQCm1UBt9spR%40tamriel.snowman.net>
Precedence: bulk


--4GcfYaGCt2KsK7LC
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

Greetings,

* Justin Pryzby (pryzby@telsasoft.com) wrote:
> On Thu, Mar 09, 2023 at 09:34:10AM -0500, Stephen Frost wrote:
> > * Nathan Bossart (nathandbossart@gmail.com) wrote:
> > > 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 enu=
m.  There
> > > >>> might be little practical difference, but since there are only th=
ree
> > > >>> possible values, I wonder if it'd be better form to make it an en=
um.
> > > >>=20
> > > >> It takes more code to write as an enum - see 002.txt.  I'm not con=
vinced
> > > >> this is better.
> > > >>=20
> > > >> But your comment made me fix its <type>, and reconsider the string=
s,
> > > >> which I changed to active=3D{unknown/true/false} rather than {unk/=
on/off}.
> > > >> It could also be active=3D{unknown/yes/no}...
> > > >=20
> > > > I think unknown/true/false is fine.  I'm okay with using a string i=
f no one
> > > > else thinks it should be an enum (or a bool).
> > >=20
> > > There's been no response for this, so I guess we can proceed with a s=
tring
> > > GUC.
> >=20
> > Just happened to see this and I'm not really a fan of this being a
> > string when it's pretty clear that's not what it actually is.
>=20
> I don't understand what you mean by that.
> Why do you say it isn't a string ?

Because it's clearly a yes/no, either the server is currently running
with huge pages, or it isn't.  That's the definition of a boolean.
Sure, anything can be cast to text but when there's a data type that
fits better, that's almost uniformly better to use.

> > > +        Reports whether huge pages are in use by the current instanc=
e.
> > > +        See <xref linkend=3D"guc-huge-pages"/> for more information.
> > >=20
> > > I still think we should say "server" in place of "current instance" h=
ere.
> >=20
> > We certainly use 'server' a lot more in config.sgml than we do
> > 'instance'.  "currently running server" might be closer to how we
> > describe a running PG system in other parts (we talk about "currently
> > running server processes", "while the server is running", "When running
> > a standby server", "when the server is running"; "instance" is used much
> > less and seems to more typically refer to 'state of files on disk' in my
> > reading vs. 'actively running process' though there's some of each).
>=20
> I called it "instance" since the GUC has no meaning when it's not
> running.  I'm fine to rename it to "running server".

Great, I do think that would match better with the rest of the
documentation.

> > > +		{"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
> > > +		},
> > >=20
> > > I don't think we need to use GUC_RUNTIME_COMPUTED.  'postgres -C' see=
ms to
> > > always report "unknown" for this GUC, so all this would do is cause t=
hat
> > > command to error unnecessarily when the server is running.
> >=20
> > ... or we could consider adjusting things to actually try the mmap() and
> > find out if we'd end up with huge pages or not.
>=20
> That seems like a bad idea, since it might work one moment and fail one
> moment later.  If we could tell in advance whether it was going to work,
> we wouldn't be here, and probably also wouldn't have invented
> huge_pages=3Dtrue.

Sure it might ... but I tend to disagree that it's actually all that
likely for it to end up being as inconsistent as that and it'd be nice
to be able to see if the server will end up successfully starting (for
this part, at least), or not, when configured with huge pages set to on,
or if starting with 'try' is likely to result in it actually using huge
pages, or not.

> > > 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 se=
em
> > > tremendously obvious.
> >=20
> > If we could get rid of that case and just make this a boolean, that
> > seems like it'd really be the best answer.
> >=20
> > To that end- perhaps this isn't appropriate as a GUC at all?  Why not
> > have this just be a system information function?  Functionally it really
> > provides the same info- if the server is running then you get back
> > either true or false, if it's not then you can't call it but that's
> > hardly different from an unknown or error result.
>=20
> We talked about making it a function ~6 weeks ago.

Oh, good, glad I'm not the only one to have thought of that.

> Is there an agreement to use a function, instead ?

Looking back at the arguments for having it be a GUC ... I just don't
really see any of them as very strong.  Not that I feel super strongly
about it being a function either, but it's certainly not a configuration
variable and it also isn't really available with postgres -C (and
therefore doesn't actually go along with how the *size GUCs work).  It's
literally information about the running system that the user might be
curious about ... and that sure seems to fit pretty cleanly under
'System Information Functions'.

Thanks,

Stephen

--4GcfYaGCt2KsK7LC
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCgAdFiEEwf6gbxKhD863zrx/7WyKOINHZFUFAmQKO1EACgkQ7WyKOINH
ZFUNdg//TO8kdYZIn8MdbAlTOgfpt5jgDDxqE4KKv+z7YAzaoYuEwUnTdrUeNmC1
dQ0uUyvodfj4canZHYG3g+YjuAxfb59vhlnBARYYSPKalnjBITj7AnGR8WrsANkl
teMWK5ZOCLHs5PC88AscKA7/C9XdV3aOM+d3BF5cyFE2rTAPEvCR2r7czPAM0tFn
qo/DRMqpuF9Fe8YiAMhnd/ftSKDhMe+uxQclC7WYN/yfwjejczi2Lktyl4cC1Bzg
3ohPlbJSKYKXuicPhJS8b2+z/XoYiwRCotp+DHqSsFAEEZIpG7DTNJixB+2oML1J
vHSFw7tpIJ/dkAO9dCcg8d2yfUj5cWWK0j/KmbhyAptxYHJfK3C7FPI+qGQSNA6H
HzVaea436x4z+8qrkHHjeQTgfmg3FpoKwqWiRhAurgqgtAjKooEJ/yDbpQXtuQ3s
mmGyUhWZME2nBFJtDNj38SR5MhdSvSw65aGkguGJfg53gt9dZBwGCWU7qI7zYu0t
M0c5e+oHVg/ueRbmUlokBdn6vvUo12Y1As0fuySV0EBFWGhyYHnWb7Msn4/Y0sYI
USAeOxX2G+kBcmUf+UpmGR6gPeR5Q+qiVFUuxHwvA/09YW9KRdXGNK0juNPvvfTU
syOtYNBhuWqYw7y94VO/OOVSDI80XrsxfiZ1CYjsLhYIxbtTF4k=
=DivQ
-----END PGP SIGNATURE-----

--4GcfYaGCt2KsK7LC--