Re: [PROPOSAL] Termination of Background Workers for ALTER/DROP DATABASE - Mailing list pgsql-hackers

From Michael Paquier
Subject Re: [PROPOSAL] Termination of Background Workers for ALTER/DROP DATABASE
Date
Msg-id aOctHMnSZcREh0BR@paquier.xyz
Whole thread Raw
In response to RE: [PROPOSAL] Termination of Background Workers for ALTER/DROP DATABASE  ("Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>)
Responses RE: [PROPOSAL] Termination of Background Workers for ALTER/DROP DATABASE
List pgsql-hackers
On Thu, Oct 09, 2025 at 02:28:56AM +0000, Hayato Kuroda (Fujitsu) wrote:
> Sorry for posting many times. I noticed that CountOtherDBBackends() can be 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 = $node->basedir();
+my $tablespace = "$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)
     memset(&worker, 0, sizeof(worker));
     worker.bgw_flags = 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 database
+sub launch_bgworker
+{
+    my ($node, $database) = @_;
+
+    # Launch a background worker on the given database
+    my $result = $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

Attachment

pgsql-hackers by date:

Previous
From: Jeff Davis
Date:
Subject: Re: Clarification on Role Access Rights to Table Indexes
Next
From: David Rowley
Date:
Subject: Re: [PATCH] Add tests for Bitmapset