Date: Mon, 6 Sep 2021 11:27:35 +0900
From: Michael Paquier <michael@paquier.xyz>
To: "Bossart, Nathan" <bossartn@amazon.com>
Cc: Justin Pryzby <pryzby@telsasoft.com>,
	Andres Freund <andres@anarazel.de>,
	Magnus Hagander <magnus@hagander.net>,
	Mark Dilger <mark.dilger@enterprisedb.com>,
	Don Seiler <don@seiler.us>,
	"pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Subject: Re: Estimating HugePages Requirements?
Message-ID: <YTV8l8V+uKVcTVnV@paquier.xyz>
References: <20210828035722.GQ26465@telsasoft.com>
 <82E1E183-691F-479E-B661-9BE8EC70BD21@amazon.com>
 <YSyIz86xUwpbP+LU@paquier.xyz>
 <48795AC4-289F-4CDE-88F5-4FD1ED2BD3DD@amazon.com>
 <YS8jgN37QvlrKnzQ@paquier.xyz>
 <298B6D88-7B71-4E1B-BFC4-E7BF3A32960D@amazon.com>
 <YTCCXOEdjbN9Sl+5@paquier.xyz>
 <23F67FC0-E432-4324-BEA4-F99B126510EA@amazon.com>
 <YTF+PTIwtDghavEc@paquier.xyz>
 <7ED221D0-0D75-46F8-8D6C-8AF70D8EABE7@amazon.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha256;
	protocol="application/pgp-signature"; boundary="mAzEGLBLNe2BZ1mH"
Content-Disposition: inline
In-Reply-To: <7ED221D0-0D75-46F8-8D6C-8AF70D8EABE7@amazon.com>
Archived-At: 
 <https://www.postgresql.org/message-id/YTV8l8V%2BuKVcTVnV%40paquier.xyz>
Precedence: bulk


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

On Fri, Sep 03, 2021 at 05:36:43PM +0000, Bossart, Nathan wrote:
> On 9/2/21, 6:46 PM, "Michael Paquier" <michael@paquier.xyz> wrote:
> You bring up an interesting point about _PG_init().  Presently, you
> can safely assume that the data directory is locked during _PG_init(),
> so there's no need to worry about breaking something on a running
> server.  I don't know how important this is.  Most _PG_init()
> functions that I've seen will define some GUCs, request some shared
> memory, register some background workers, and/or install some hooks.
> Those all seem like safe things to do, but I wouldn't be at all
> surprised to hear examples to the contrary.  In any case, it looks
> like the current ordering of these two steps has been there for 15+
> years.

Yeah.  What you are describing here matches what I have seen in the
past and what we do in core for _PG_init().  Now extensions developers
could do more fancy things, like dropping things on-disk to check the
load state, for whatever reasons.  And things could break in such
cases.  Perhaps people should not do that, but it is no fun either to
break code that has been working for years even if that's just a major
upgrade.

+        * We skip this step if we are just going to print a GUC's value an=
d exit
+        * a few steps down.
         */
-       CreateDataDirLockFile(true);
+       if (output_config_variable =3D=3D NULL)
+               CreateDataDirLockFile(true);

Anyway, 0002 gives me shivers.

> If this is a concern, one option would be to disallow running "-C
> shared_memory_size" on running servers.  That would have to extend to
> GUCs like data_checksums and huge_pages_required, too.

Just noting this bit from 0003 that would break without 0002:
-$ <userinput>pmap 4170 | awk '/rw-s/ &amp;&amp; /zero/ {print $2}'</userin=
put>
-6490428K
+$ <userinput>postgres -D $PGDATA -C shared_memory_size</userinput>
+6339

>> 0001, that refactors the calculation of the shmem size into a
>> different routine, is fine as-is, so I'd like to move on and apply
>> it.
>=20
> Sounds good to me.

Applied this one.

Without concluding on 0002 yet, another thing that we could do is to
just add the GUCs.  These sound rather useful on their own (mixed
feelings about huge_pages_required but I can see why it is useful to
avoid the setup steps and the backend already grabs this information),
particularly when it comes to cloned setups that share a lot of
properties.
--
Michael

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

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

iQIzBAEBCAAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmE1fJYACgkQnvQgOdby
QH3t3g//U67dk/bPJwnRoE4uPfnkFcYoqfSSZN5GsWHPRrfRApwLoe+oB/m5voBw
wHnhDdFrq7P1mEuL8G2W4TTQg64A9BdA+0TjgC8MYGFFLepAm7tbu9YOGeVsDICN
u4mCtT7M3I6C0tlQcZgaJ3f5cRzZApiyKCQs/wkNXl8emDx1AlmnZXQAL2eFPn4a
B0F7xPdkxgu0wq1hYcwal50Qxc1LAwa1WKNZDuwRm5raxfke7lzDgZA+eCAPx/7f
/xIY9LAIDl7STx5AIeSHG9di0YIHE6gsFARW6nGyaL5OBuz4PWq2J+O4yRcVCdsa
pUe4MlxtRTQWmT+3i6n68zWw3FmmLc/nRqYj5rZd15gQD3v/k6Yuy6QbU5VeDoAZ
hteeWlHWbcuIpTmwxff02wE2+D/Y/HbsiGJ4HeH2F9GgrLGKXMwx8Yj2RG8dTxuU
IG4aAy2qODmJkX0ZiKs9krMWy4IIYl0OBddlm1kVh8rV1gOJlBGOOa5vOMwEvber
tQpq3gmTwMGUZoFU48JHZBeg/zQLZThxC4jGSIEZs1apzGLQdeyDppjuA0OtTG0n
rD3+0hHuffICF/vCbqR5mu0uTIjYAWLWfHyIzEGYKNcsQB6UhejjaDeyD7tBhTBU
Hcfcu6QsQKr6vWvdprs1BDdvgUXXSnkM6Tmi0L5oKajbA7Poro8=
=EnWS
-----END PGP SIGNATURE-----

--mAzEGLBLNe2BZ1mH--