Date: Mon, 30 Aug 2021 16:29:19 +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: <YSyIz86xUwpbP+LU@paquier.xyz>
References: <0545F7B3-70C0-4DE8-8C85-EAFE6113B7EE@amazon.com>
 <YSiKS58gSXDpj6NR@paquier.xyz>
 <CABUevEwGHcsmp4rgZdcbinZOdEs_cSCabihDvRty=0zz1H95kw@mail.gmail.com>
 <20210827184648.d3jbv73pgyccqwkz@alap3.anarazel.de>
 <071DC95B-4CA8-4D2B-BD24-1A66B3EFFE28@amazon.com>
 <20210827193813.oqo5lamvyzahs35o@alap3.anarazel.de>
 <0D1645F3-ADFD-4B2F-9F34-961BDFBB881B@amazon.com>
 <YSmYq5Z4GAf8NzxM@paquier.xyz>
 <20210828035722.GQ26465@telsasoft.com>
 <82E1E183-691F-479E-B661-9BE8EC70BD21@amazon.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha256;
	protocol="application/pgp-signature"; boundary="G2nUSsBgiGsrZoza"
Content-Disposition: inline
In-Reply-To: <82E1E183-691F-479E-B661-9BE8EC70BD21@amazon.com>
Archived-At: 
 <https://www.postgresql.org/message-id/YSyIz86xUwpbP%2BLU%40paquier.xyz>
Precedence: bulk


--G2nUSsBgiGsrZoza
Content-Type: multipart/mixed; boundary="/+o9XxN+yDGX74SE"
Content-Disposition: inline


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

On Sat, Aug 28, 2021 at 05:36:37AM +0000, Bossart, Nathan wrote:
> Attached is a hacky attempt at adding a shared_memory_size GUC in a
> way that could be used with -C.  This should include the amount of
> shared memory requested by extensions, too.  As long as huge_page_size
> is nonzero, it seems easy enough to provide the number of huge pages
> needed as well.

Yes, the implementation is not good.  The key thing is that by wanting
to support shared_memory_size with the -C switch of postgres, we need
to call process_shared_preload_libraries before output_config_variable.=20
This additionally means to call ApplyLauncherRegister() before that so
as all the bgworker slots are not taken first.  Going through
_PG_init() also means that we'd better use ChangeToDataDir()
beforehand.

Attached is a WIP to show how the order of the operations could be
changed, as that's easier to grasp.  Even if we don't do that, having
the GUC and the refactoring of CalculateShmemSize() would still be
useful, as one could just query an existing instance for an estimation
of huge pages for a cloned one.

The GUC shared_memory_size should have GUC_NOT_IN_SAMPLE and
GUC_DISALLOW_IN_FILE, with some documentation, of course.  I added the
flags to the GUC, not the docs.   The code setting up the GUC is not
good either.  It would make sense to just have that in a small wrapper
of ipci.c, perhaps.
--
Michael

--/+o9XxN+yDGX74SE
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment;
	filename="v2-0001-Add-shared_memory_size-GUC.patch"
Content-Transfer-Encoding: quoted-printable

=46rom e2345206dacec8b4454c3b429d0cfac01d5a0a58 Mon Sep 17 00:00:00 2001
=46rom: Michael Paquier <michael@paquier.xyz>
Date: Mon, 30 Aug 2021 16:24:59 +0900
Subject: [PATCH] Add shared_memory_size GUC

---
 src/include/storage/ipc.h           |   1 +
 src/backend/postmaster/postmaster.c |  60 +++++++-----
 src/backend/storage/ipc/ipci.c      | 142 ++++++++++++++++------------
 src/backend/utils/misc/guc.c        |  13 +++
 4 files changed, 135 insertions(+), 81 deletions(-)

diff --git a/src/include/storage/ipc.h b/src/include/storage/ipc.h
index 753a6dd4d7..80e191d407 100644
--- a/src/include/storage/ipc.h
+++ b/src/include/storage/ipc.h
@@ -77,6 +77,7 @@ extern void check_on_shmem_exit_lists_are_empty(void);
 /* ipci.c */
 extern PGDLLIMPORT shmem_startup_hook_type shmem_startup_hook;
=20
+extern Size CalculateShmemSize(int *num_semaphores);
 extern void CreateSharedMemoryAndSemaphores(void);
=20
 #endif							/* IPC_H */
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/p=
ostmaster.c
index 9c2c98614a..0d3b4b1be0 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -893,6 +893,44 @@ PostmasterMain(int argc, char *argv[])
 	if (!SelectConfigFiles(userDoption, progname))
 		ExitPostmaster(2);
=20
+	/* Verify that DataDir looks reasonable */
+	checkDataDir();
+
+	/* Check that pg_control exists */
+	checkControlFile();
+
+	/* And switch working directory into it */
+	ChangeToDataDir();
+
+	/*
+	 * Register the apply launcher.  Since it registers a background worker,
+	 * it needs to be called before InitializeMaxBackends(), and it's probably
+	 * a good idea to call it before any modules had chance to take the
+	 * background worker slots.
+	 */
+	ApplyLauncherRegister();
+
+	/*
+	 * Process any libraries that should be preloaded at postmaster start.
+	 * Thie happens before running -C, so as it is possible to get an
+	 * estimation of the total shared memory size allocated to this system,
+	 * accounting for the portion from loaded libraries.
+	 */
+	process_shared_preload_libraries();
+
+	/* XXX: this should be moved around */
+	{
+		char buf[64];
+		Size size;
+
+		size =3D CalculateShmemSize(NULL);
+		size +=3D (1024 * 1024) - 1;
+		size /=3D (1024 * 1024);
+
+		sprintf(buf, "%lu MB", size);
+		SetConfigOption("shared_memory_size", buf, PGC_INTERNAL, PGC_S_OVERRIDE);
+	}
+
 	if (output_config_variable !=3D NULL)
 	{
 		/*
@@ -907,15 +945,6 @@ PostmasterMain(int argc, char *argv[])
 		ExitPostmaster(0);
 	}
=20
-	/* Verify that DataDir looks reasonable */
-	checkDataDir();
-
-	/* Check that pg_control exists */
-	checkControlFile();
-
-	/* And switch working directory into it */
-	ChangeToDataDir();
-
 	/*
 	 * Check for invalid combinations of GUC settings.
 	 */
@@ -996,19 +1025,6 @@ PostmasterMain(int argc, char *argv[])
 	 */
 	LocalProcessControlFile(false);
=20
-	/*
-	 * Register the apply launcher.  Since it registers a background worker,
-	 * it needs to be called before InitializeMaxBackends(), and it's probably
-	 * a good idea to call it before any modules had chance to take the
-	 * background worker slots.
-	 */
-	ApplyLauncherRegister();
-
-	/*
-	 * process any libraries that should be preloaded at postmaster start
-	 */
-	process_shared_preload_libraries();
-
 	/*
 	 * Initialize SSL library, if specified.
 	 */
diff --git a/src/backend/storage/ipc/ipci.c b/src/backend/storage/ipc/ipci.c
index 3e4ec53a97..b225b1ee70 100644
--- a/src/backend/storage/ipc/ipci.c
+++ b/src/backend/storage/ipc/ipci.c
@@ -75,6 +75,87 @@ RequestAddinShmemSpace(Size size)
 	total_addin_request =3D add_size(total_addin_request, size);
 }
=20
+/*
+ * CalculateShmemSize
+ *		Calculates the amount of shared memory and number of semaphores needed.
+ *
+ * If num_semaphores is not NULL, it will be set to the number of semaphor=
es
+ * required.
+ *
+ * Note that this function freezes the additional shared memory request si=
ze
+ * from loadable modules.
+ */
+Size
+CalculateShmemSize(int *num_semaphores)
+{
+	Size size;
+	int numSemas;
+
+	/* Compute number of semaphores we'll need */
+	numSemas =3D ProcGlobalSemas();
+	numSemas +=3D SpinlockSemas();
+
+	/* Return the number of semaphores if requested by the caller */
+	if (num_semaphores)
+		*num_semaphores =3D numSemas;
+
+	/*
+	 * Size of the Postgres shared-memory block is estimated via moderately-
+	 * accurate estimates for the big hogs, plus 100K for the stuff that's too
+	 * small to bother with estimating.
+	 *
+	 * We take some care to ensure that the total size request doesn't overfl=
ow
+	 * size_t.  If this gets through, we don't need to be so careful during t=
he
+	 * actual allocation phase.
+	 */
+	size =3D 100000;
+	size =3D add_size(size, PGSemaphoreShmemSize(numSemas));
+	size =3D add_size(size, SpinlockSemaSize());
+	size =3D add_size(size, hash_estimate_size(SHMEM_INDEX_SIZE,
+											 sizeof(ShmemIndexEnt)));
+	size =3D add_size(size, dsm_estimate_size());
+	size =3D add_size(size, BufferShmemSize());
+	size =3D add_size(size, LockShmemSize());
+	size =3D add_size(size, PredicateLockShmemSize());
+	size =3D add_size(size, ProcGlobalShmemSize());
+	size =3D add_size(size, XLOGShmemSize());
+	size =3D add_size(size, CLOGShmemSize());
+	size =3D add_size(size, CommitTsShmemSize());
+	size =3D add_size(size, SUBTRANSShmemSize());
+	size =3D add_size(size, TwoPhaseShmemSize());
+	size =3D add_size(size, BackgroundWorkerShmemSize());
+	size =3D add_size(size, MultiXactShmemSize());
+	size =3D add_size(size, LWLockShmemSize());
+	size =3D add_size(size, ProcArrayShmemSize());
+	size =3D add_size(size, BackendStatusShmemSize());
+	size =3D add_size(size, SInvalShmemSize());
+	size =3D add_size(size, PMSignalShmemSize());
+	size =3D add_size(size, ProcSignalShmemSize());
+	size =3D add_size(size, CheckpointerShmemSize());
+	size =3D add_size(size, AutoVacuumShmemSize());
+	size =3D add_size(size, ReplicationSlotsShmemSize());
+	size =3D add_size(size, ReplicationOriginShmemSize());
+	size =3D add_size(size, WalSndShmemSize());
+	size =3D add_size(size, WalRcvShmemSize());
+	size =3D add_size(size, PgArchShmemSize());
+	size =3D add_size(size, ApplyLauncherShmemSize());
+	size =3D add_size(size, SnapMgrShmemSize());
+	size =3D add_size(size, BTreeShmemSize());
+	size =3D add_size(size, SyncScanShmemSize());
+	size =3D add_size(size, AsyncShmemSize());
+#ifdef EXEC_BACKEND
+	size =3D add_size(size, ShmemBackendArraySize());
+#endif
+
+	/* freeze the addin request size and include it */
+	addin_request_allowed =3D false;
+	size =3D add_size(size, total_addin_request);
+
+	/* might as well round it off to a multiple of a typical page size */
+	size =3D add_size(size, 8192 - (size % 8192));
+
+	return size;
+}
=20
 /*
  * CreateSharedMemoryAndSemaphores
@@ -102,65 +183,8 @@ CreateSharedMemoryAndSemaphores(void)
 		Size		size;
 		int			numSemas;
=20
-		/* Compute number of semaphores we'll need */
-		numSemas =3D ProcGlobalSemas();
-		numSemas +=3D SpinlockSemas();
-
-		/*
-		 * Size of the Postgres shared-memory block is estimated via
-		 * moderately-accurate estimates for the big hogs, plus 100K for the
-		 * stuff that's too small to bother with estimating.
-		 *
-		 * We take some care during this phase to ensure that the total size
-		 * request doesn't overflow size_t.  If this gets through, we don't
-		 * need to be so careful during the actual allocation phase.
-		 */
-		size =3D 100000;
-		size =3D add_size(size, PGSemaphoreShmemSize(numSemas));
-		size =3D add_size(size, SpinlockSemaSize());
-		size =3D add_size(size, hash_estimate_size(SHMEM_INDEX_SIZE,
-												 sizeof(ShmemIndexEnt)));
-		size =3D add_size(size, dsm_estimate_size());
-		size =3D add_size(size, BufferShmemSize());
-		size =3D add_size(size, LockShmemSize());
-		size =3D add_size(size, PredicateLockShmemSize());
-		size =3D add_size(size, ProcGlobalShmemSize());
-		size =3D add_size(size, XLOGShmemSize());
-		size =3D add_size(size, CLOGShmemSize());
-		size =3D add_size(size, CommitTsShmemSize());
-		size =3D add_size(size, SUBTRANSShmemSize());
-		size =3D add_size(size, TwoPhaseShmemSize());
-		size =3D add_size(size, BackgroundWorkerShmemSize());
-		size =3D add_size(size, MultiXactShmemSize());
-		size =3D add_size(size, LWLockShmemSize());
-		size =3D add_size(size, ProcArrayShmemSize());
-		size =3D add_size(size, BackendStatusShmemSize());
-		size =3D add_size(size, SInvalShmemSize());
-		size =3D add_size(size, PMSignalShmemSize());
-		size =3D add_size(size, ProcSignalShmemSize());
-		size =3D add_size(size, CheckpointerShmemSize());
-		size =3D add_size(size, AutoVacuumShmemSize());
-		size =3D add_size(size, ReplicationSlotsShmemSize());
-		size =3D add_size(size, ReplicationOriginShmemSize());
-		size =3D add_size(size, WalSndShmemSize());
-		size =3D add_size(size, WalRcvShmemSize());
-		size =3D add_size(size, PgArchShmemSize());
-		size =3D add_size(size, ApplyLauncherShmemSize());
-		size =3D add_size(size, SnapMgrShmemSize());
-		size =3D add_size(size, BTreeShmemSize());
-		size =3D add_size(size, SyncScanShmemSize());
-		size =3D add_size(size, AsyncShmemSize());
-#ifdef EXEC_BACKEND
-		size =3D add_size(size, ShmemBackendArraySize());
-#endif
-
-		/* freeze the addin request size and include it */
-		addin_request_allowed =3D false;
-		size =3D add_size(size, total_addin_request);
-
-		/* might as well round it off to a multiple of a typical page size */
-		size =3D add_size(size, 8192 - (size % 8192));
-
+		/* Compute the size of the shared-memory block */
+		size =3D CalculateShmemSize(&numSemas);
 		elog(DEBUG3, "invoking IpcMemoryCreate(size=3D%zu)", size);
=20
 		/*
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 467b0fd6fe..872bdc1c8e 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -620,6 +620,8 @@ char	   *pgstat_temp_directory;
=20
 char	   *application_name;
=20
+int			shmem_size_mb;
+
 int			tcp_keepalives_idle;
 int			tcp_keepalives_interval;
 int			tcp_keepalives_count;
@@ -2337,6 +2339,17 @@ static struct config_int ConfigureNamesInt[] =3D
 		NULL, NULL, NULL
 	},
=20
+	{
+		{"shared_memory_size", PGC_INTERNAL, RESOURCES_MEM,
+			gettext_noop("Shows the amount of shared memory allocated by the server=
 (rounded up to the nearest MB)."),
+			NULL,
+			GUC_NOT_IN_SAMPLE | GUC_DISALLOW_IN_FILE | GUC_UNIT_MB
+		},
+		&shmem_size_mb,
+		0, 0, INT_MAX,
+		NULL, NULL, NULL
+	},
+
 	{
 		{"temp_buffers", PGC_USERSET, RESOURCES_MEM,
 			gettext_noop("Sets the maximum number of temporary buffers used by each=
 session."),
--=20
2.33.0


--/+o9XxN+yDGX74SE--

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

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

iQIzBAEBCAAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmEsiM8ACgkQnvQgOdby
QH2X7xAAgAZnZtpc1KIq/MNM03daPtcd8RtRFwDXpiFdwLG6A1AX+2sfE0NBAFWa
wzZeLYmes+h30xIFtxr3TmCY/el4iKfNkijYyjqF1cS8Qklwy26JfR55CUGb+kVH
U+zBEHewd+FtPfBKjUYt5Kry57JW6lKLZMiC0xuDXUAGV7UqGZbK5AEimz5ppaX6
ek71TF3eBa2LuqeIrhm6iTPOImGyAg5bse+W1fbpDhGdqLPAnySV7hGfsHMJy5Qv
R2sRePFfvFuuLHkFF/mzzJRNC83d1b4IO/TA5hyLvk6FtP0cZRqmSBeAf7CjZ8UH
HKijAd1/5jRdDCEv6ndWlsbgSQwtr56aCQze1by6uIdY2PbLrYazNUkQ8VBwL1Q/
ZL+L9dxzX7TEvgQK/3IVUKOyZVu6eu1aNoPqNwWBcE8sgc8XDpN/hkncpIja4CHE
2Fq2sXjLlob/IToHF6M2JA786XMRvpZ//WtTlti6oAW/WNd83u/0LrfXPNu0BxpZ
54a+PRXJnDZrKH0lNGoTHGxWlMigo/Gz7u5kT2Q4S0BB+3ZThwveQQ7XJNIZEZ7Y
bjK2a4KHIWGE//zV+Bg37xES8fWHZCend7jp6qu7JAzAlzV3y6x58veaV5LVFTN5
YM7UJ6WDZAk305tDWpK7xZBykjMa4L2gmVe7uHm4sgeiSHqAMu4=
=8mRD
-----END PGP SIGNATURE-----

--G2nUSsBgiGsrZoza--