Feedback-ID: i0fe9450f:Fastmail
Date: Thu, 20 Jul 2023 12:55:43 +0900
From: Michael Paquier <michael@paquier.xyz>
To: Masahiro Ikeda <ikedamsh@oss.nttdata.com>
Cc: pgsql-hackers@lists.postgresql.org
Subject: Re: Support worker_spi to execute the function dynamically.
Message-ID: <ZLiwP5vrT/SENzim@paquier.xyz>
References: <d30d3ea7d21cb7c9e1e3cc47e301f1b6@oss.nttdata.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="54O1t6EzTU9AyGZk"
Content-Disposition: inline
In-Reply-To: <d30d3ea7d21cb7c9e1e3cc47e301f1b6@oss.nttdata.com>
Archived-At: <https://www.postgresql.org/message-id/ZLiwP5vrT/SENzim%40paquier.xyz>
Precedence: bulk


--54O1t6EzTU9AyGZk
Content-Type: multipart/mixed; boundary="LGF9d42mdQj4Kx5k"
Content-Disposition: inline


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

On Thu, Jul 20, 2023 at 11:15:51AM +0900, Masahiro Ikeda wrote:
> While I'm working on the thread[1], I found that the function of
> worker_spi module fails if 'shared_preload_libraries' doesn't have
> worker_spi.

I guess that you were patching worker_spi to register dynamically a
wait event and embed that in a TAP test or similar without loading it
in shared_preload_libraries?  FWIW, you could use a trick like what I
am attaching here to load a wait event dynamically with the custom
wait event API.  You would need to make worker_spi_init_shmem() a bit
more aggressive with an extra hook to reserve a shmem area size, but
that's enough to show the custom wait event in the same backend as the
one that launches a worker_spi dynamically, while demonstrating how
the API can be used in this case.

> In my understanding, the restriction is not required. So, I think it's
> better to change the behavior.
> (v1-0001-Support-worker_spi-to-execute-the-function-dynamical.patch)
>=20
> What do you think?

+1.  I'm OK to lift this restriction with a SIGHUP GUC for the
database name and that's not a pattern to encourage in a template
module.  Will do so, if there are no objections.
--
Michael

--LGF9d42mdQj4Kx5k
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="0001-Add-tweak-to-allow-worker_spi-to-register-wait_event.patch"
Content-Transfer-Encoding: quoted-printable

=46rom 2fcd773cd4009cffd626640e1553d4efdceb0777 Mon Sep 17 00:00:00 2001
=46rom: Michael Paquier <michael@paquier.xyz>
Date: Thu, 20 Jul 2023 12:48:07 +0900
Subject: [PATCH] Add tweak to allow worker_spi to register wait_event
 dynamically

---
 src/test/modules/worker_spi/worker_spi.c | 35 +++++++++++++++++++++++-
 1 file changed, 34 insertions(+), 1 deletion(-)

diff --git a/src/test/modules/worker_spi/worker_spi.c b/src/test/modules/wo=
rker_spi/worker_spi.c
index b87d6c75d8..4e3da93129 100644
--- a/src/test/modules/worker_spi/worker_spi.c
+++ b/src/test/modules/worker_spi/worker_spi.c
@@ -48,11 +48,20 @@ PG_FUNCTION_INFO_V1(worker_spi_launch);
=20
 PGDLLEXPORT void worker_spi_main(Datum main_arg) pg_attribute_noreturn();
=20
+static void worker_spi_init_shmem(void);
+
 /* GUC variables */
 static int	worker_spi_naptime =3D 10;
 static int	worker_spi_total_workers =3D 2;
 static char *worker_spi_database;
=20
+typedef struct WorkerSpiState
+{
+	uint32 wait_event_info;
+} WorkerSpiState;
+
+/* the pointer to the shared memory */
+static WorkerSpiState *worker_spi_state =3D NULL;
=20
 typedef struct worktable
 {
@@ -149,6 +158,8 @@ worker_spi_main(Datum main_arg)
 	/* We're now ready to receive signals */
 	BackgroundWorkerUnblockSignals();
=20
+	worker_spi_init_shmem();
+
 	/* Connect to our database */
 	BackgroundWorkerInitializeConnection(worker_spi_database, NULL, 0);
=20
@@ -199,7 +210,7 @@ worker_spi_main(Datum main_arg)
 		(void) WaitLatch(MyLatch,
 						 WL_LATCH_SET | WL_TIMEOUT | WL_EXIT_ON_PM_DEATH,
 						 worker_spi_naptime * 1000L,
-						 WAIT_EVENT_EXTENSION);
+						 worker_spi_state->wait_event_info);
 		ResetLatch(MyLatch);
=20
 		CHECK_FOR_INTERRUPTS();
@@ -346,6 +357,26 @@ _PG_init(void)
 	}
 }
=20
+static void
+worker_spi_init_shmem(void)
+{
+	bool found =3D false;
+
+	LWLockAcquire(AddinShmemInitLock, LW_EXCLUSIVE);
+	worker_spi_state =3D ShmemInitStruct("worker_spi",
+									   sizeof(WorkerSpiState),
+									   &found);
+	if (!found)
+	{
+		/* First time through ... */
+		worker_spi_state->wait_event_info =3D WaitEventExtensionNew();
+	}
+	LWLockRelease(AddinShmemInitLock);
+
+	WaitEventExtensionRegisterName(worker_spi_state->wait_event_info,
+								   "worker_spi_custom");
+}
+
 /*
  * Dynamically launch an SPI worker.
  */
@@ -358,6 +389,8 @@ worker_spi_launch(PG_FUNCTION_ARGS)
 	BgwHandleStatus status;
 	pid_t		pid;
=20
+	worker_spi_init_shmem();
+
 	memset(&worker, 0, sizeof(worker));
 	worker.bgw_flags =3D BGWORKER_SHMEM_ACCESS |
 		BGWORKER_BACKEND_DATABASE_CONNECTION;
--=20
2.40.1


--LGF9d42mdQj4Kx5k--

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

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

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmS4sD8ACgkQnvQgOdby
QH204BAAju+EVKQ5Co/wu7zQwI63QSMclyKtakqomrMD5foDMtI/lwPZbqcGXV8E
Vd1h4/go0j6z3nyigf6P+uFlSiXAYG9LYn1czCzTUHz7DHsblZVZrj6A+OTfJN9Q
N1uYe3492MLkMWt9K3X6Ekt/v1Oisr8ev6GsWjNbxOB++vGf8FRcMxPqFlo+4bB2
WxtMnuw3H8qqK8Zl9XQ2sF3NxwDSA77bAiNpj5tnjbO92twP49Nd2sLrwpEuH9dF
1Xb3TVOpU3dpLaLkRSXVZMK+SXzsbKYteGKmg1+rdQ52DiQZ4W16KudN9Gw2/TpY
5k5zOUDlacAmwrzINgiD5meGeRQINlbxxiqfYaRj8AHcEG0xnDeSgRWuJ8+guWjh
g22QO7YXHZKu2qxyQRtpmiwMin6HURS8Ci0yKNcRZlrseeSeWD9TlTch40ikzmw7
vKfm0NioEHZoOb25xGTyVS+1uNencWgpNdVZk+wymzuFtlPhjrlSxShK7l7E/9ob
MQdImsMHLLuE7ELcN2Y1jksJUO/sSAdd7fFaJVF9sIC0WEW9KlFVoBvhCjT4Vbif
c7HFrQWOpBtsvY642R4kZd2LCLfIGoKMh+kH3O17DSKV6PRkfwGVZTO8x0BegKrL
cY1Z1sqYTxQ86xvnz42dI6RZIMqjbtRTPs5V4rufQ9dS9zrT09o=
=PC41
-----END PGP SIGNATURE-----

--54O1t6EzTU9AyGZk--