Feedback-ID: i0fe9450f:Fastmail
Date: Thu, 9 Oct 2025 12:33:48 +0900
From: Michael Paquier <michael@paquier.xyz>
To: "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>
Cc: 'Peter Smith' <smithpb2250@gmail.com>,
	"Aya Iwata (Fujitsu)" <iwata.aya@fujitsu.com>,
	pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: Re: [PROPOSAL] Termination of Background Workers for ALTER/DROP
 DATABASE
Message-ID: <aOctHMnSZcREh0BR@paquier.xyz>
References: 
 <OSCPR01MB149662AEA64F4E66F494C2584F5E3A@OSCPR01MB14966.jpnprd01.prod.outlook.com>
 <aOM02tVFtXmuR0M1@paquier.xyz>
 <OS7PR01MB119648846BA926C096CE016B6EAE3A@OS7PR01MB11964.jpnprd01.prod.outlook.com>
 <CAHut+Pt4Tn1bQYCsYeUt_gtcSB-KOTtRB70SLghkpsjfKGsm7w@mail.gmail.com>
 <OS7PR01MB119642CFE4F4DCF7FBBD040BBEAE0A@OS7PR01MB11964.jpnprd01.prod.outlook.com>
 <CAHut+Pu-nxNice547eGEW2O3hdRcFbPYWF4HiqktZO19X3Ah-g@mail.gmail.com>
 <OS7PR01MB11964628A74081DFCA442CBADEAE1A@OS7PR01MB11964.jpnprd01.prod.outlook.com>
 <CAHut+PuuOKKW2oDT0Z8q+UXsiteS67j6G6075FC3aAxeR0cxHQ@mail.gmail.com>
 <OSCPR01MB14966FC50CFB457938763AE09F5EEA@OSCPR01MB14966.jpnprd01.prod.outlook.com>
 <OSCPR01MB14966B98E5119EBB261024ED2F5EEA@OSCPR01MB14966.jpnprd01.prod.outlook.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="pghrxNMd99t7fBe5"
Content-Disposition: inline
In-Reply-To: 
 <OSCPR01MB14966B98E5119EBB261024ED2F5EEA@OSCPR01MB14966.jpnprd01.prod.outlook.com>
Archived-At: 
 <https://www.postgresql.org/message-id/aOctHMnSZcREh0BR%40paquier.xyz>
Precedence: bulk


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

On Thu, Oct 09, 2025 at 02:28:56AM +0000, Hayato Kuroda (Fujitsu) wrote:
> Sorry for posting many times. I noticed that CountOtherDBBackends() can b=
e called
> while creating the database. Should we also mention and test the case?

How would you test that?  A bgworker would not be able to connect to
the database that's being created.

The implementation done in v4 is a nice simplification compared to the
original proposal, nice.  I've gone through the patch and here are
some comments.

+my $basedir =3D $node->basedir();
+my $tablespace =3D "$basedir/tablespace";

We could use a temporary folder for the tablespaces.  I've always
prefered this practice.  That's a bit, feel free to ignore this one,
what you are doing is not wrong, either.

+     <term><literal>BGWORKER_EXIT_AT_DATABASE_DROP</literal></term>

Perhaps BGWORKER_EXIT_AT_DATABASE_CHANGE?  DROP is incorrect, as the
database could be renamed or moved, as well.

+ * Exit the bgworker when its database is dropped, renamed, or moved.
+ * Requires BGWORKER_SHMEM_ACCESS and BGWORKER_BACKEND_DATABASE_CONNECTION.
+ */
+#define BGWORKER_EXIT_AT_DATABASE_DROP                         0x0004

We could enforce this rule with an elog(ERROR) or an assert, perhaps?

@@ -407,7 +407,8 @@ worker_spi_launch(PG_FUNCTION_ARGS)=20
 	memset(&worker, 0, sizeof(worker));
 	worker.bgw_flags =3D BGWORKER_SHMEM_ACCESS |
-		BGWORKER_BACKEND_DATABASE_CONNECTION;
+		BGWORKER_BACKEND_DATABASE_CONNECTION |
+		BGWORKER_EXIT_AT_DATABASE_DROP;

I would suggest to make this part optional, with an argument that
uses a default value to false (as in "do-not-set the flag") that can
be given by the callers of worker_spi_launch().  Let's also add one
bgworker that's used in your series of tests, and check that it is
*not* cancelled when the flag is not set.

+# Ensure the worker_spi dynamic worker is launched on the specified databa=
se
+sub launch_bgworker
+{
+	my ($node, $database) =3D @_;
+
+	# Launch a background worker on the given database
+	my $result =3D $node->safe_psql(
+		$database, qq(
+        SELECT worker_spi_launch(4, oid) IS NOT NULL

I'd recommend to make the worker number an argument of this function,
and also do things so as the log_contains() call is able to check that
the worker with the matching number is loged, rather than rely on
"worker_spi dynamic" for all the comparisons.  This is relevant to be
able to mix multiple workers at the same time in the tests.
--
Michael

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

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

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmjnLRwACgkQnvQgOdby
QH16CA/8CPjVfnJ+L5HzI6hJ9olkxcfjcU63uFrkx6IPBW8hsx2GkMIfmd2R9LCv
EkOgcrShDKJmYVGUmuWm/FgoK7dmGKoamYrcrOP+W6gezIS2PCTPisalUdNruJby
7gaX/eyfT2EoHa3rxKd/dRB5+r3IDunagK566XKX4xucWrbWC1VJVQ81IkRMUADu
NaTw5g5aURAUmNkgTvmpMOsMzD7AOIjtHS0I8oR0zz4vIYORbkZud/uHoX68ynZL
0Dy2lcTplzcfylA3RzP1v3H8BQPIFl3z/4k0L2nLR/8Mi5RbEWDJbgv4RptTv1Xs
EjlahckzdJ5MuZbYqmrZmK43Vba894OiO2I24E87ivvYwQVyslmilHq/npxYmq+O
Rl+SgMfQ3r/8ybct6DoL8iO+aqyKuxZgaE1JLvt+tLW7pAjdddfowk1JVaHImZPo
kOu5/yBX+pIBDqV8yGwqARn7UwCaaMgMJLZv+xEBMfJ051v1A/ut8NQN03tACuQ7
J6u910wTp4o5/l890F4BPytLkYBkj8tYZaxdTUdmU+5Vj0Xyp+aX6cWXQwxAWm1h
emEb04aA4cVRNIUJ/dwIC7ACXpRE2ZJHZBKTM3uWCDeGbZ/owttvwpMa0HkLMsAi
R0MTy84+4241fjRJ11SHVn92rPgpnNdRC2rbHBfKmhRDpliuv/g=
=rs2e
-----END PGP SIGNATURE-----

--pghrxNMd99t7fBe5--