Subject: [PROPOSAL] Termination of Background Workers for ALTER/DROP DATABASE

From: "Aya Iwata (Fujitsu)" <[email-masked]>
Date: Fri, 3 Oct 2025 09:27:23 +0000

Hi,

Background
==========
If the background workers connect to databases, some database-related commands
like ALTER DATABASE RENAME and ALTER DATABASE SET TABLESPACE cannot be done.
Users must do DROP EXTENSION related with workers, or terminate them by themselves
if they want to drop or alter the database.

Proposal
========
Based on above, I would like to propose to terminate background workers automatically
when such SQLs are executed.

This feature allows the DBMS daemon to send a termination signal to background workers
created by users currently operating on the database when executing commands that make
significant changes to the database.

To receive the termination signal, the background worker must call the
AcceptBackgroundWorkerCancel() function, using the database's OID and a flag
indicating whether to terminate. This means existing background worker processes
will not abruptly terminate.

This termination occurs when executing the DROP DATABASE, ALTER DATABASE RENAME TO,
or ALTER DATABASE SET TABLE SPACE commands, which check the existence of processes.

When a user creates a background worker to perform some data processing or monitoring,
and wants to terminate it along with the database deletion, this feature enables
achieving that goal.

The test set for this feature will be shared later.

How do you feel? Your feedback is very welcome.

Regards,
Aya Iwata
Fujitsu Limited

---

From: "Hayato Kuroda (Fujitsu)" <[email-masked]>
Date: Mon, 6 Oct 2025 01:59:08 +0000

Dear Iwata-san,

>Background

Per my understanding, we already have a facility that terminates a background
worker, TerminateBackgroundWorker(). So, I'm afraid your proposal has already
been done by combining this function and ProcessUtility_hook.

So, is the main benefit of the patch to shorten extensions codes which uses
bgworker?

Best regards,
Hayato Kuroda
FUJITSU LIMITED

---

From: Michael Paquier <[email-masked]>
Date: Mon, 6 Oct 2025 12:17:46 +0900

> been done by combining this function and ProcessUtility_hook.

The main take that I am getting here from Iwata-san is that this would
lead to less code duplication.  Another item, which you are not
mentioning, is that this would be more flexible with bgworkers that
have been starting dynamically, where shared_preload_libraries may not
be used, still a bgworker would need to react.  So the suggestion of a
new API to control if a bgworker should be stopped like any other
backend when there is a database activity worth it is a good one, as
long as it is in line with what we do with normal backends.

AcceptBackgroundWorkerCancel() is going backwards, IMO.  Wouldn't it
be better to pass it down as an option in bgw_flags, to mark that a
bgworker connected to a database can be shutdown due to the effect of
a database getting dropped or moved?  There could be an argument
behind using bgw_extra, but that would not be in line with the
database connection argument which is a state defined when a bgworker
is registered.

> So, is the main benefit of the patch to shorten extensions codes which uses

I'd ask for the addition of tests when it comes to such a facility,
and your proposal has none of that.  I would suggest worker_spi with
an option that can be passed to worker_spi_launch(). 
--
Michael

---

From: "Hayato Kuroda (Fujitsu)" <[email-masked]>
Date: Mon, 6 Oct 2025 09:15:53 +0000

Dear Michael,

> long as it is in line with what we do with normal backends.

Okay, so this proposal has the advantage that we can terminate workers, even if the
extensions do not control workers on the shared memory, right?

> a database getting dropped or moved?

+1 to extend the bgw_flags.

Best regards,
Hayato Kuroda
FUJITSU LIMITED

---

From: "Aya Iwata (Fujitsu)" <[email-masked]>
Date: Mon, 6 Oct 2025 11:21:02 +0000

Hi,

Thank you for your comments.

> lead to less code duplication.  

Yeah. My company sometimes wrote extensions which uses background workers, 
and it was painful to implement the same part every time.

> long as it is in line with what we do with normal backends.

Thanks for the agreement. Thus I want to proceed the patch.

> database connection argument which is a state defined when a bgworker

I updated my patch using bgw_flags to set whether accept to terminate bgworker or not.
And I also removed AcceptBackgroundWorkerCancel() function. 
Please check my attached patch.

> an option that can be passed to worker_spi_launch().

I added the TAP test using worker_spi too.

Regards,
Aya Iwata
Fujitsu Limited

---

From: "Hayato Kuroda (Fujitsu)" <[email-masked]>
Date: Tue, 7 Oct 2025 02:00:18 +0000

Dear Iwata-san,

Thanks for updating the patch.

> Please check my attached patch.

```
+/*
+ * Cancel background workers.
+ */
+void
+CancelBackgroundWorkers(Oid databaseId, int cancel_flags)
```

Do we still need the cancel_flags? I cannot find other reasons to terminate
workers. Also the things I don't like is that BGWORKER_CANCEL_ADMIN_COMMANDS must
have the same value as BGWORKER_EXIT_AT_DATABASE_DROP. Only one flag exists but
it has 0x0004. Can we remove the argument and flags from the patch?

[1]: https://www.postgresql.org/message-id/OSCPR01MB149662AEA64F4E66F494C2584F5E3A%40OSCPR01MB14966.jpnprd01.prod.outlook.com

Best regards,
Hayato Kuroda
FUJITSU LIMITED

---

From: Peter Smith <[email-masked]>
Date: Tue, 7 Oct 2025 14:12:06 +1100

Hi,

Here are some more minor review comments:

======
doc/src/sgml/bgworker.sgml

1. Typo?

s/damon/daemon/

======
src/backend/postmaster/bgworker.c

2.
+void
+CancelBackgroundWorkers(Oid databaseId, int cancel_flags)
+{
+ int slotno;
+ bool signal_postmaster = false;
+
+ LWLockAcquire(BackgroundWorkerLock, LW_EXCLUSIVE);
+
+ for (slotno = 0; slotno < BackgroundWorkerData->total_slots; ++slotno)
+ {
+ BackgroundWorkerSlot *slot = &BackgroundWorkerData->slot[slotno];
+
+ /* Check worker slot. */
+ if (!slot->in_use)
+ continue;
+
+ /* 1st, check cancel flags. */
+ if ((slot->worker.bgw_flags & BGWORKER_EXIT_AT_DATABASE_DROP) & cancel_flags)
+ {
+ PGPROC     *proc = BackendPidGetProc(slot->pid);
+
+ if (!proc)
+ continue;
+
+ /* 2nd, compare databaseId. */
+ if (proc->databaseId == databaseId)
+ {
+ /*
+ * Set terminate flag in shared memory, unless slot has
+ * been reused.
+ */
+ slot->terminate = true;
+ signal_postmaster = true;
+ }
+ }
+ }

2a.
Declare slotno as a 'for' loop variable.

~

2b.
There seem to be excessive conditions in the code. Is it better to
restructure with less, like:

for (int slotno = 0; ...)
{
  ...

  if (!slot->in_use)
    continue;

  if (slot flags are not set to drop)
    continue;
  proc = BackendPidGetProc(slot->pid);
  if (proc && proc->databaseId == databaseId)
  {
    slot->terminate = true;
    signal_postmaster = true;
  }
}

======
Kind Regards,
Peter Smith.
Fujitsu Australia

---

From: "Aya Iwata (Fujitsu)" <[email-masked]>
Date: Tue, 7 Oct 2025 10:33:16 +0000

Hi,

Thank you for your comments. I updated patch to v0003.

> it has 0x0004. Can we remove the argument and flags from the patch?

One reason for adding these flags was that I considered a case where
we might not want to allow all worker terminations during database deletion,
even when the BGWORKER_EXIT_AT_DATABASE_DROP flag is set.
However, This might be a rare case. Therefore, I removed these flags.

> doc/src/sgml/bgworker.sgml

Thank you. Yes, it is a typo. I fixed this.

> Declare slotno as a 'for' loop variable.

Thank you. I fixed this.

>     signal_postmaster = true;

Thank you. I fixed this, too.

Regards,
Aya Iwata
Fujitsu Limited

---

From: "Hayato Kuroda (Fujitsu)" <[email-masked]>
Date: Tue, 7 Oct 2025 11:49:13 +0000

Dear Iwata-san,

Thanks for updating the patch. Comments:

```

The comment has less meaning. How about:
"Skip if the slot is not used"

```
+		/* 1st, check cancel flags. */
+		if (slot->worker.bgw_flags & BGWORKER_EXIT_AT_DATABASE_DROP)
```

Missing update 2b [1]. Also, since cancel flag does not exist anymore, the comment should be
Updated. How about something like:
"Skip if the background worker does not want to exit"

```
+			/* 2nd, compare databaseId. */
+			if (proc && proc->databaseId == databaseId)
```

Here should describes what are you trying to do. How about something like:
Checks the connecting database of the worker, and instruct the postmaster to terminate it if needed

```
+		/*
+		 * Cancel background workers by admin commands.
+		 */
+		CancelBackgroundWorkers(databaseId);
```

Since we removed the flag, the comment is outdated.

```
-
 typedef void (*bgworker_main_type) (Datum main_arg);
```

This change is not related with this patch.

```
@@ -361,7 +361,8 @@ _PG_init(void)
 	/* set up common data for all our workers */
 	memset(&worker, 0, sizeof(worker));
 	worker.bgw_flags = BGWORKER_SHMEM_ACCESS |
-		BGWORKER_BACKEND_DATABASE_CONNECTION;
+		BGWORKER_BACKEND_DATABASE_CONNECTION |
+		BGWORKER_EXIT_AT_DATABASE_DROP;
```

The new flag was added to both static and dynamic background workers. So, how about
testing both? I think it is enough to use one of case, like ALTER DATABASE SET TABLESPACE.

[1]: https://www.postgresql.org/message-id/CAHut%2BPt4Tn1bQYCsYeUt_gtcSB-KOTtRB70SLghkpsjfKGsm7w%40mail.gmail.com

Best regards,
Hayato Kuroda
FUJITSU LIMITED

---

From: Peter Smith <[email-masked]>
Date: Wed, 8 Oct 2025 08:39:43 +1100

HI Iwata-San,

Here are some more review comments for v3.

======

1.
+      <para>
+       <indexterm><primary>BGWORKER_EXIT_AT_DATABASE_DROP</primary></indexterm>
+       Requests to terminate background worker when the database connected by
+       the background worker is changed. DBMS daemon can issue a termination
+       signal to the background worker.
+       This occurs only when significant changes affecting the entire database
+       take place.
+       Specifically, major changes include when the <command>DROP
DATABASE</command>,
+       <command>ALTER DATABASE RENAME TO</command>, and
+       <command>ALTER DATABASE SET TABLESPACE</command> commands are executed.
+      </para>

Here is a reworded version of that for your consideration
(AI-generated -- pls verify for correctness!):

<para>
 <indexterm><primary>BGWORKER_EXIT_AT_DATABASE_DROP</primary></indexterm>
 Requests termination of the background worker when the database it is
 connected to undergoes significant changes. The postmaster will send a
 termination signal to the background worker when any of the following
 commands are executed: <command>DROP DATABASE</command>,
 <command>ALTER DATABASE RENAME TO</command>, or
 <command>ALTER DATABASE SET TABLESPACE</command>.
</para>

======

2.
+ for (int slotno = 0; slotno < BackgroundWorkerData->total_slots; ++slotno)
+ {

IMO, most of those comments do not have any benefit because they only
repeat what is already obvious from the code.

2a.
+ /* Check worker slot. */
+ if (!slot->in_use)
Remove that one. It is the same as the code.

~

2b.
+ /* 1st, check cancel flags. */
+ if (slot->worker.bgw_flags & BGWORKER_EXIT_AT_DATABASE_DROP)
Remove that one. It is the same as the code

~

2c.
+ /* 2nd, compare databaseId. */
+ if (proc && proc->databaseId == databaseId)
Remove that one. It is the same as the code.

~

2d.
+ /*

This comment is a bit strange -- It seems slightly misplaced. IIUC,
the "unless slot has been reused" really is referring to the earlier
"slot->in_use". This whole comment may be better put immediately above
the 'for' loop as a short summary of the whole logic.

======
src/include/postmaster/bgworker.h

3.
+/*
+ * This flag means the bgworker must be exit when the connecting database is
+ * being dropped or moved.
+ * It requires both BGWORKER_SHMEM_ACCESS and
+ * BGWORKER_BACKEND_DATABASE_CONNECTION were passed too.
+ */

Not English. Needs rewording. Consider something like this:

/*
 * Exit the bgworker when its database is dropped, renamed, or moved.
 * Requires BGWORKER_SHMEM_ACCESS and BGWORKER_BACKEND_DATABASE_CONNECTION.
 */

======
Kind Regards,
Peter Smith.
Fujitsu Australia

---

From: "Aya Iwata (Fujitsu)" <[email-masked]>
Date: Wed, 8 Oct 2025 12:26:05 +0000

Hi Kuroda-san, Peter-san,

Thank you for your review comments! I updated patch to v0004.

> -----Original Message-----
> From: Kuroda, Hayato/黒田 隼人 <[email-masked]>
> Sent: Tuesday, October 7, 2025 8:49 PM

> Checks the connecting database of the worker, and instruct the postmaster to

This comment has been removed. We can know code's intent without comments.

> Since we removed the flag, the comment is outdated.

Thank you. I changed this comment:
"if set the bgw_flags, cancel background workers."

> This change is not related with this patch.

I removed this change.

> testing both? I think it is enough to use one of case, like ALTER DATABASE SET

I removed this BGWORKER_EXIT_AT_DATABASE_DROP from the patch.

> -----Original Message-----
> From: Peter Smith <[email-masked]>

>  <command>ALTER DATABASE SET TABLESPACE</command>.

Thank you. I checked and replaced the doc.

> Remove that one. It is the same as the code.

I removed this comment.

> Remove that one. It is the same as the code

I removed this comment, too.

> Remove that one. It is the same as the code.

I removed this comment, too.

> the 'for' loop as a short summary of the whole logic.

I put this comment to before the for 'loop'.
And I changed comment like this:

/*
 * Set terminate flag in shared memory, unless slot has
 * been used.
 */

> BGWORKER_BACKEND_DATABASE_CONNECTION.

Thank you. I changed the source code comments based on this comment.
 
Regards,
Aya Iwata
Fujitsu Limited

---

From: Peter Smith <[email-masked]>
Date: Thu, 9 Oct 2025 09:18:00 +1100

Hi Iwata-San,

Some v4 comments.

======

1.
+ /*
+ * Set terminate flag in shared memory, unless slot has
+ * been used.
+ */
+ for (int slotno = 0; slotno < BackgroundWorkerData->total_slots; ++slotno)
+ {
+ PGPROC     *proc;
+ BackgroundWorkerSlot *slot = &BackgroundWorkerData->slot[slotno];
+
+ if (!slot->in_use)
+ continue;
+
+ if (!(slot->worker.bgw_flags & BGWORKER_EXIT_AT_DATABASE_DROP))
+ continue;
+
+ proc = BackendPidGetProc(slot->pid);
+
+ if (proc && proc->databaseId == databaseId)
+ {
+ slot->terminate = true;
+ signal_postmaster = true;
+ }
+ }

1a.
It's not clear to me what you were trying to convey by saying "unless
slot has been used" in the comment. Maybe you meant "unless slot is
not in use", but is that useful even to say? Anyway, the comment as-is
seems incorrect.

~

1b.
Sorry for wavering on this, but now that I see the resulting v4 code,
I feel we don't really need any of those 'continues', and more if
conditions can be combined. It becomes simpler. See if you agree.

SUGGESTION:

for (int slotno ...)
{
  if (slot->in_use && (slot->worker.bgw_flags & BGWORKER_EXIT_AT_DATABASE_DROP))
  {
    PGPROC *proc = BackendPidGetProc(slot->pid);
    if (proc && proc->databaseId == databaseId)
    {
      slot->terminate = true;
      signal_postmaster = true;
    }
  }
}

======
src/backend/storage/ipc/procarray.c

2.
+ /*
+ * if set the bgw_flags, cancel background workers.
+ */
+ CancelBackgroundWorkers(databaseId);
+

I was wondering about this function name "CancelXXX" -- do you
"cancel" a worker, or do you "terminate" it?

Isn't it better to name this new function more like the
existing/similar TerminateBackgroundWorker() function?

E.g. consider the following:

/*
 * Terminate all background workers for this database, if
 * they had requested it (BGWORKER_EXIT_AT_DATABASE_DROP).
 */
TerminateBackgroundWorkersForDB(databaseId);

======
Kind Regards,
Peter Smith.
Fujitsu Australia

---

From: "Hayato Kuroda (Fujitsu)" <[email-masked]>
Date: Thu, 9 Oct 2025 02:05:36 +0000

Dear Peter, Iwata-san,

> not in use", but is that useful even to say? Anyway, the comment as-is

Agreed to update the comment. How about:
Iterate through slots, looking for workers who connects to the given database.

> conditions can be combined. It becomes simpler. See if you agree.

Ether way is fine for me.

> TerminateBackgroundWorkersForDB(databaseId);

The code comment looks OK. Regarding the function name, I want to propose
an alternative - TerminateBackgroundWorkersByOid().
Core codes have already had several xxxByOid() functions.

Best regards,
Hayato Kuroda
FUJITSU LIMITED

---

From: "Hayato Kuroda (Fujitsu)" <[email-masked]>
Date: Thu, 9 Oct 2025 02:28:56 +0000

Dear Iwata-san,

Sorry for posting many times. I noticed that CountOtherDBBackends() can be called
while creating the database. Should we also mention and test the case?

Best regards,
Hayato Kuroda
FUJITSU LIMITED

---

From: Michael Paquier <[email-masked]>
Date: Thu, 9 Oct 2025 12:33:48 +0900

> 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) 

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

---

From: "Hayato Kuroda (Fujitsu)" <[email-masked]>
Date: Thu, 9 Oct 2025 04:00:24 +0000

Dear Michael,

> the database that's being created.

Sorry for missing words. Per my understanding, CountOtherDBBackends() in createdb()
ensures that there are no active connections of the source database. If there is
a connection to a database, we cannot create another database with TEMPALATE clause:

```
postgres=# CREATE DATABASE new TEMPLATE postgres ;
ERROR:  source database "postgres" is being accessed by other users
DETAIL:  There is 1 other session using the database.
```

Based on that, I imagined that we could launch a bgworker and create another database
by using template. Or is it already handled?

Best regards,
Hayato Kuroda
FUJITSU LIMITED

---

From: Michael Paquier <[email-masked]>
Date: Thu, 9 Oct 2025 13:37:16 +0900

On Thu, Oct 09, 2025 at 04:00:24AM +0000, Hayato Kuroda (Fujitsu) wrote:
> Based on that, I imagined that we could launch a bgworker and create
> another database by using template. Or is it already handled?

Oh, I was not following you here.  Yes, we should have something with
a bgworker connected to a source and make sure that it disconnects
when create a new database with this source as template.

However, could there be more to consider here?  Contrary to DROP
DATABASE, where we require the drop to be done by the owner of the
database (or a superuser), CREATE DATABASE has less requirements: it
is fine for a role to create a database if they have the CREATEDB
rights.  If we allow bgworkers to be cancelled when the database they
are connected to is used as a source, that may be disruptive, so we
had better document precisely the behavior of the flag and what users
should expect from it when set.  We are already killing autovacuum
workers when they are connected to a database used as a source anyway
(since 4abd7b49f1e9 from 2008), so I am worrying for nothing and it is
actually better to apply the same rule across the board.  Let's just
document very clearly how the bgworker will react when the cancel flag
is set in all these cases.
--
Michael

---

From: "Hayato Kuroda (Fujitsu)" <[email-masked]>
Date: Thu, 9 Oct 2025 05:47:49 +0000

Dear Michael,

> should expect from it when set.

Actually, if the database is not marked as the template one, the user must be
owner of the source or superuser. Not sure there is a real case that template
database has dedicated workers, but anyway I do agree to note down this behavior.
It is surprising that creating other databases lead the process terminations.

Best regards,
Hayato Kuroda
FUJITSU LIMITED

---

From: "Aya Iwata (Fujitsu)" <[email-masked]>
Date: Thu, 9 Oct 2025 13:09:23 +0000

Hi,

I updated the patch to v0005. 

> Sent: Thursday, October 9, 2025 7:18 AM

> not in use", but is that useful even to say? Anyway, the comment as-is

I changed this comment. I use Kuroda-san's comment. Thank you. )
"Iterate through slots, looking for workers who connects to the given database."

> conditions can be combined. It becomes simpler. See if you agree.

I changed if condition code.

> TerminateBackgroundWorkersForDB(databaseId);

I changed this name to "TerminateBackgroudWorkerByOid"

> From: Michael Paquier <[email-masked]>
> Sent: Thursday, October 9, 2025 12:34 PM

> what you are doing is not wrong, either.

I use tempdir to create directory for the tablespace.

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

I started think it does not a strict condition. 
If the worker does not connect to databases,
the BGWORKER_EXIT_AT_DATABASE_DROP(CHANGE) flag will be never checked.
Therefore, I have changed this comment as follows:
" No-op if BGWORKER_BACKEND_DATABASE_CONNECTION are not specified."

> *not* cancelled when the flag is not set.

I fixed this to added allow_termination flag and *not* canceled test.
However this test takes at least 5 sec. because of "for loop" in CountOtherDBBackends().

> able to mix multiple workers at the same time in the tests.

I fixed test code. Thank you for your advice.
It is better to used wait_for_log because it takes time for the log to output.
And we added test for "CREATE DATABASE TEMPLATE " command too.

Regards,
Aya Iwata
Fujitsu Limited

---

From: Peter Smith <[email-masked]>
Date: Fri, 10 Oct 2025 09:56:53 +1100

Hi Iwata-San,

Some v5 comments.

======

1.
+    <varlistentry>
+     <term><literal>BGWORKER_EXIT_AT_DATABASE_CHANGE</literal></term>
+     <listitem>
+      <para>
+       <indexterm><primary>BGWORKER_EXIT_AT_DATABASE_CHANGE</primary></indexterm>
+       Requests termination of the background worker when the database it is
+       connected to undergoes significant changes. The postmaster will send a
+       termination signal to the background worker when any of the following
+       commands are executed: <command>DROP DATABASE</command>,
+       <command>ALTER DATABASE RENAME TO</command>, or
+       <command>ALTER DATABASE SET TABLESPACE</command>.
+       When <command>CREATE DATABASE TEMPLATE</command> command is executed,
+       background workers which connected to target template database
are terminated.
+       If <literal>BGWORKER_SHMEM_ACCESS</literal> and
+       <literal>BGWORKER_BACKEND_DATABASE_CONNECTION</literal> are not using,
+       nothing happens.
+      </para>
+     </listitem>
+    </varlistentry>
+

1a.
The newly added part in v5 needs some brush-up.

SUGGESTION:

<para>
 <indexterm><primary>BGWORKER_EXIT_AT_DATABASE_CHANGE</primary></indexterm>
 Requests termination of the background worker when its connected database
 undergoes significant changes. The postmaster sends a termination signal
 when any of these commands affect the worker's database:
 <command>DROP DATABASE</command>,
 <command>ALTER DATABASE RENAME TO</command>,
 <command>ALTER DATABASE SET TABLESPACE</command>, or
 <command>CREATE DATABASE</command> (when the worker is connected to the
 template database).
 This flag requires both <literal>BGWORKER_SHMEM_ACCESS</literal> and
 <literal>BGWORKER_BACKEND_DATABASE_CONNECTION</literal>.
</para>

~

1b.
Elsewhere on this docs page, there are detailed discussions about
process termination (e.g. search "terminate"), as well as referring to
the function TerminateBackgroundWorker().

You only documented the new flag, but do we also need to make changes
in the rest of this page to mention the new way for process
termination?

======

TerminateBackgroundWorkersByOid:

2.
+/*
+ * Cancel background workers.
+ */
+void
+TerminateBackgroundWorkersByOid(Oid databaseId)

Let's also remove that word "Cancel" from the function comment and add
some more details.

SUGGESTION:
Terminate all background workers connected to the given database, if
they had requested it.

~~~

3.
+ /*
+ * Iterate through slots, looking for workers
+ * who connects to the given database.
+ */

"workers who connects" (??)

SUGGESTION
Iterate through slots, looking for workers connected to the given database.

======

4.
+ /*
+ * Terminate all background workers for this database, if
+ * they had requested it (BGWORKER_EXIT_AT_DATABASE_DROP)
+ */
+ TerminateBackgroundWorkersByOid(databaseId);
+

The comment is out-of-date now, because the flag name was changed to
BGWORKER_EXIT_AT_DATABASE_CHANGE.

======

5.
+/* Cancel background workers. */
+extern void TerminateBackgroundWorkersByOid(Oid databaseId);

There is already a commented extern for the
TerminateBackgroundWorker() function. IMO, just put this extern
alongside that one. You don't need a new comment.

~~~

6. The header comment in this file refers to the
TerminateBackgroundWorker() function. Probably, you need to check that
carefully to see if this new function should also be mentioned there.

======

FYI, I attached a v5 top-up diff for (some of) my above review
comments in case it helps.

======
Kind Regards,
Peter Smith.
Fujitsu Australia

---

From: Chao Li <[email-masked]>
Date: Fri, 10 Oct 2025 10:03:06 +0800

Hi Iwata-san,

A few comments:

> On Oct 9, 2025, at 21:09, Aya Iwata (Fujitsu) <[email-masked]> wrote:
> 
> Hi,
> 
> <v0005-0001-Allow-background-workers-to-be-terminated.patch>

1 - bgworker.sgml
```
+    <varlistentry>
+       background workers which connected to target template database are terminated.

This paragraph has several English problems:

* “Undergoes significant changes” sounds vague, better to say “is dropped, renamed or moved to a different tablespace”.
* “When CREATE DATABASE TEMPLATE command is executed” - missing articles.
* “background workers which connected to target template database” - wrong tense/relative pronoun.
* “are not using” should be “are not used” or “are not set”

Suggested revision:

```
<indexterm><primary>BGWORKER_EXIT_AT_DATABASE_CHANGE</primary></indexterm>
Requests termination of the background worker when the database it is
connected to is dropped, renamed, or moved to a different tablespace.
In these cases, the postmaster will send a termination signal to the
background worker when any of the following commands are executed:
<command>DROP DATABASE</command>, <command>ALTER DATABASE RENAME TO</command>,
or <command>ALTER DATABASE SET TABLESPACE</command>.

When a <command>CREATE DATABASE ... TEMPLATE ...</command> command is executed,
background workers connected to the template database used as the source are
also terminated.

If neither <literal>BGWORKER_SHMEM_ACCESS</literal> nor
<literal>BGWORKER_BACKEND_DATABASE_CONNECTION</literal> is set, this action
has no effect.
```

2 - bgworker.h
```
+#define BGWORKER_EXIT_AT_DATABASE_CHANGE                         0x0004
```

You are using white-spaces between the macro name and value, that’s why 0x0004 looks not aligned in my IDE. I think you should use a couple tabs between them.

3 - bgworker.h
```

An OID can represent a lot of things. So, instead of suggesting the OID type by parameter name, I wonder if it is better do that with the function name, like TerminateBgWorkersByDbOid(Oid oid)

4 - procarray.c
```
+		/*

I wonder if the correct parameter should be BGWORKER_EXIT_AT_DATABASE_CHANGE in the comment, as you are adding BGWORKER_EXIT_AT_DATABASE_CHANGE with this patch.

5 - bgworker.c
```
+/*

I think the function name is more descriptive than the function comment. So, please either remove function comment or enhance it.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

---

From: "Aya Iwata (Fujitsu)" <[email-masked]>
Date: Fri, 10 Oct 2025 09:04:54 +0000

Hi,

Thank you for your comments. I updated patch to v0006.

> From: Peter Smith mailto:[email-masked]
> Sent: Friday, October 10, 2025 7:57 AM

> comments in case it helps.

Thank you very much. I've updated the patch using the attached diff file.

> From: Chao Li <[email-masked]> 
> Sent: Friday, October 10, 2025 11:03 AM

> * “are not using” should be “are not used” or “are not set”

Thank you for your comment an suggested revision. I changed .sgml documentation.

>You are using white-spaces between the macro name and value, that’s why 0x0004 looks not aligned in my IDE. I think you should use a couple tabs between them.

Thank you. I fixed this white-spaces (using pgindent).

> An OID can represent a lot of things. So, instead of suggesting the OID type by parameter name, I wonder if it is better do that with the function name, like TerminateBgWorkersByDbOid(Oid oid)

After receiving your comment, I checked other functions and there is no other examples like XXOid function in the code.
If this function use only here, original code is using databaseId in argument and it clear what Oid is.
I think original name is fine because it's not a function that's called much elsewhere.

> I wonder if the correct parameter should be BGWORKER_EXIT_AT_DATABASE_CHANGE in the comment, as you are adding BGWORKER_EXIT_AT_DATABASE_CHANGE with this patch.

Thank you. I fixed this comment.

>  I think the function name is more descriptive than the function comment. So, please either remove function comment or enhance it.

I changed this function comment:
" Terminate all background workers connected to the given database, if they had requested it."

Regards,
Aya Iwata
Fujitsu Limited.

---

From: Andrei Lepikhov <[email-masked]>
Date: Fri, 10 Oct 2025 13:05:04 +0200

On 10/10/2025 11:04, Aya Iwata (Fujitsu) wrote:
> Thank you for your comments. I updated patch to v0006.My company also employs a large number of background workers. I also 
reviewed your patch.
It looks nice to be committed. The only question I have is whether to 
make sending a termination signal a default behaviour and let the flag 
deactivate it.

-- 
regards, Andrei Lepikhov,
pgEdge

---

From: Peter Smith <[email-masked]>
Date: Mon, 13 Oct 2025 09:28:40 +1100

Hi Iwata-San,

Some v6 comments.

======

1.
+      <para>
+       <indexterm><primary>BGWORKER_EXIT_AT_DATABASE_CHANGE</primary></indexterm>
+       Requests termination of the background worker when its
connected database
+       is dropped, renamed, or moved to a different tablespace.
+       In these cases, the postmaster will send a termination signal to the
+       background worker when any of the following commands are executed:
+       <command>DROP DATABASE</command>,
+       <command>ALTER DATABASE RENAME TO</command>,
+       <command>ALTER DATABASE SET TABLESPACE</command>, or
+       <command>CREATE DATABASE</command> (when the worker is connected to the
+       template database).
+       This flag requires both <literal>BGWORKER_SHMEM_ACCESS</literal> and
+       <literal>BGWORKER_BACKEND_DATABASE_CONNECTION</literal>.
+      </para>

IMO, below is an improved wording for this:

<para>
 <indexterm><primary>BGWORKER_EXIT_AT_DATABASE_CHANGE</primary></indexterm>
 Requests termination of the background worker when its connected database is
 dropped, renamed, moved to a different tablespace, or used as a template for
 <command>CREATE DATABASE</command>. Specifically, the postmaster sends a
 termination signal when any of these commands affect the worker's database:
 <command>DROP DATABASE</command>,
 <command>ALTER DATABASE RENAME TO</command>,
 <command>ALTER DATABASE SET TABLESPACE</command>, or
 <command>CREATE DATABASE</command>.
 Requires both <literal>BGWORKER_SHMEM_ACCESS</literal> and
 <literal>BGWORKER_BACKEND_DATABASE_CONNECTION</literal>.
</para>

======

+
+
+/*
+ * Terminate all background workers connected to the given database, if they
+ * had requested it.
+ */
+void
+TerminateBackgroundWorkersByOid(Oid databaseId)

Only 1 blank line is needed here.

======

+/*
+ * Exit the bgworker when its database is dropped, renamed, or moved.
+ * No-op if BGWORKER_BACKEND_DATABASE_CONNECTION is not specified.
+ */
+#define BGWORKER_EXIT_AT_DATABASE_CHANGE 0x0004
+

That double-negative comment seems awkward. IMO, positive statements
are clearer. Also, do you think you should mention
BGWORKER_SHMEM_ACCESS, or was that deliberately omitted because
BGWORKER_BACKEND_DATABASE_CONNECTION requires that?

e.g. The suggested comment below is more closely aligned with the documentation.

SUGGESTION:
/*
 * Exit the bgworker when its database is dropped, renamed, moved to a
 * different tablespace, or used as a template for CREATE DATABASE.
 * Requires BGWORKER_SHMEM_ACCESS and BGWORKER_BACKEND_DATABASE_CONNECTION.
 */

======
src/test/modules/worker_spi/t/002_worker_terminate.pl

+sub launch_bgworker
+{
+ my ($node, $database, $testcase, $allow_terminate) = @_;
+ my $offset = -s $node->logfile;

Would '$request_terminate' be a more correct name for the $allow_terminate var?

======
src/test/modules/worker_spi/worker_spi.c

+ bool allow_termination = PG_GETARG_BOOL(4);

  memset(&worker, 0, sizeof(worker));
  worker.bgw_flags = BGWORKER_SHMEM_ACCESS |
  BGWORKER_BACKEND_DATABASE_CONNECTION;
+
+ if (allow_termination)
+ worker.bgw_flags |= BGWORKER_EXIT_AT_DATABASE_CHANGE;
+

Would 'request_termination' be a more correct name for this new var?

======
Kind Regards,
Peter Smith.
Fujitsu Australia

---

From: Peter Smith <[email-masked]>
Date: Mon, 13 Oct 2025 12:20:29 +1100

> Would 'request_termination' be a more correct name for this new var?

There's another similar parameter name that I missed in the last post.
See /src/test/modules/worker_spi/worker_spi--1.0.sql

"  allow_termination boolean DEFAULT false)"

======
Kind Regards,
Peter Smith.
Fujitsu Australia.

---

From: "Hayato Kuroda (Fujitsu)" <[email-masked]>
Date: Tue, 14 Oct 2025 04:36:01 +0000

Dear Andrei,

> The only question I have is whether to

I think it can be done in future enhancement.
We can change the default behavior based on the feedback from developers.

Best regards,
Hayato Kuroda
FUJITSU LIMITED

---

From: "Aya Iwata (Fujitsu)" <[email-masked]>
Date: Wed, 15 Oct 2025 02:48:43 +0000

Hi Peter san, 

Thank you for your comments. I updated this patch to v0007.

> Sent: Monday, October 13, 2025 10:20 AM
> To: Iwata, Aya/岩田 彩 <[email-masked]>
> Cc: Chao Li <[email-masked]>; Kuroda, Hayato/黒田 隼人
> <[email-masked]>; Michael Paquier <[email-masked]>;
> pgsql-hackers <[email-masked]>
> Subject: Re: [PROPOSAL] Termination of Background Workers for
> ALTER/DROP DATABASE
> 
> > +       This flag requires both
> <literal>BGWORKER_SHMEM_ACCESS</literal> and
> > +

I updated this .sgml file. Thank you for your advice!

> > Only 1 blank line is needed here.

I added 1 blank here.

> BGWORKER_BACKEND_DATABASE_CONNECTION.

I have replaced this code comment.

> "  allow_termination boolean DEFAULT false)"

I changed these parameter's name to "request_termination".

...
> "  allow_termination boolean DEFAULT false)"

I also fixed this, too.

Best regards,
Aya Iwata
Fujitsu Limited

---

From: Chao Li <[email-masked]>
Date: Wed, 15 Oct 2025 11:37:05 +0800

> On Oct 10, 2025, at 17:04, Aya Iwata (Fujitsu) <[email-masked]> wrote:
> 
>> 3 - bgworker.h
>> ```

By searching for “ByOid”, we can get some existing examples:

ObjectAddress
RefreshMatViewByOid(Oid matviewOid, bool is_create, bool skipData,
                    bool concurrent, const char *queryString,
                    QueryCompletion *qc)

The function name clearly tells refresh MatView by Oid, so the oid in parameter is an old of mat view.

ResultRelInfo *
ExecLookupResultRelByOid(ModifyTableState *node, Oid resultoid,
                         bool missing_ok, bool update_cache)

The function name indicates ResultRel, so the oid is a result oid.

AccessMethodInfo *
findAccessMethodByOid(Oid oid)

The function name tells to find access method, the the oid is an access method’s OID.

You can find more …

But in this patch, the function name only indeeds “terminate background workers”, while the oid is a database oid. Maybe we can rename the function to “TerminateDatabaseBgWorkersByOid()”.

Best regards,

---

From: Michael Paquier <[email-masked]>
Date: Thu, 16 Oct 2025 12:37:01 +0900

> We can change the default behavior based on the feedback from developers.

Yeah, I cannot agree with a change in the default behavior to cancel
the workers on a database touched by a database command.  This is a
behavior that exists since bgworkers are supported in tree in 9.3.  If
one is interested in making the workers more responsive, they could
just flip the flag switch.
--
Michael

---

From: Michael Paquier <[email-masked]>
Date: Thu, 16 Oct 2025 12:54:31 +0900

> Thank you for your comments. I updated this patch to v0007.

+ * Exit the bgworker when its database is dropped, renamed, moved to a
+ * different tablespace, or used as a template for CREATE DATABASE.

I don't think that we need to list all these operations in details
here. We could just say "if its database is involved in a CREATE,
ALTER or DROP database command".  The docs should provide these
details, of course.

Flag name works here.

# XXX This spends more than 5 seconds because the backend retries counting
# number of connecting processes 50 times. See CountOtherDBBackends().

And that's annoying.  Let's activate what I call the cheat mode for
this one: an injection point that, if defined, enforces a lower number
of tries when we loop over the workers to stop.  That would make the
test much faster when using a worker that should not be stopped,
without impacting the coverage.

I suspect that your new test 002_worker_terminate.pl has a race
condition in run_db_command(): are you sure that the bgworker has
enough time to be reported as stopped in the server logs once
safe_psql() finishes to run the database command given by the caller?
On very slow and/or loaded machines, particularly, that could hurt the
stability.  It seems to me that this should use a wait_for_log()
instead of a log_contains(), waiting for the worker to be reported as
stopped depending on the command executed.

Shouldn't this test also check that worker 0 (the one that does not
have the flag set) is still running at the end of the test?  I assume
that querying pg_stat_activity would be enough at the end of the
script.
--
Michael

---

From: "Aya Iwata (Fujitsu)" <[email-masked]>
Date: Thu, 16 Oct 2025 15:04:09 +0000

Hi Chao-san, Michael san

Thank you for your comments! To accept your comment, I updated patch to v0008.

> Sent: Wednesday, October 15, 2025 12:37 PM
...
> But in this patch, the function name only indeeds “terminate background workers”, while the oid is a database oid. Maybe we can rename the 
> function to “TerminateDatabaseBgWorkersByOid()”.

Thank you. I changed the function name to "'TerminateBgWorkersByDbOid".
I prefer this name because there are not official terminology "Database background worker" and it's shorter.

> Sent: Thursday, October 16, 2025 12:55 PM
...
 
> ALTER or DROP database command".  The docs should provide these

Thank you. I fixed this .h file comment.

> +#define BGWORKER_EXIT_AT_DATABASE_CHANGE       0x0004

Sorry, I cannot follow. Please tell me more details about this comment.

> without impacting the coverage.

I tried to implement your idea. Thanks Kuroda-san to help it.

> stopped depending on the command executed.

I fixed this test to use wait_for_log() instead of log_contains().

> that querying pg_stat_activity would be enough at the end of the

Added. I cannot find a good way to clarify the worker is "worker 0" from the pg_stat_activity, 
backend_type does not have the information. Thus I used the string "worker_spi dynamic" as the key.

Best regards,
Aya Iwata 
Fujitsu Limited

---

From: Peter Smith <[email-masked]>
Date: Mon, 20 Oct 2025 13:01:31 +1100

Hi Iwata-San,

Some comments for the latest v8 patch.

======

TerminateBgWorkersByBbOid:

1.
+void
+TerminateBgWorkersByDbOid(Oid oid)

Now the function name is more explicit, but that is not a good reason
to make the parameter name more vague.

IMO the parameter should still be "dbOid" or "databaseId" instead of
just "oid". (ditto for the extern in bgworker.h)

======

CountOtherDBBackends:

2.
+ /*
+ * Usually, we try 50 times with 100ms sleep between tries, making 5 sec
+ * total wait. If requested, it would be reduced to 10 times to shorten the
+ * test time.
+ */

The comment seemed vague to me. How about more like:

/*
 * Retry up to 50 times with 100ms between attempts (max 5s total).
 * Can be reduced to 10 attempts (max 1s total) to speed up tests.
 */

~~~

3.
+ for (tries = 0; tries < ntries; tries++)

'tries' can be declared as a for-loop variable.

~~~

4.
Something feels strange about this function name
(CountOtherDBBackends) which suggests it is just for counting stuff,
but in reality is more about exiting/terminating the workers. In fact
retuns a boolean, not a count. Compare this with this similarly named
"CountUserBackends" which really *is* doing what it says.

Can we give this function a better name, or is that out of scope for this patch?

======

5.
+# Firstly register an injection point to make the test faster. Normally, it
+# spends more than 5 seconds because the backend retries, counting the number
+# of connecting processes 50 times, but now the counting would be done only 10
+# times. See CountOtherDBBackends().
+$node->safe_psql('postgres', "CREATE EXTENSION injection_points;");
+$node->safe_psql('postgres',
+ "SELECT injection_points_attach('reduce-ncounts', 'error');");
+

It seemed overkill to give details about what "normally" happens. I
think it is enough to have a simple comment here:

SUGGESTION
The injection point 'reduce-ncounts' reduces the number of backend
retries, allowing for shorter test runs. See CountOtherDBBackends().

======
Kind Regards,
Peter Smith.
Fujitsu Australia

---

From: Michael Paquier <[email-masked]>
Date: Mon, 20 Oct 2025 13:32:44 +0900

> Some comments for the latest v8 patch.

The comments of Peter apply to comments and parameters.  I am not
going down to these details in this message, these can be infinitely
tuned.

The injection point integration looks correct.  You are checking the
compile flag and if the extension is available in the installation
path, which should be enough.

+   if (IS_INJECTION_POINT_ATTACHED("reduce-ncounts"))
+       ntries = 10;

1s is much faster than the default of 5s, still I am wondering if this
cannot be brought down a bit more.  Dropping the worker still around
after the first test with CREATE DATABASE works here.

+# Confirm a background worker is still running
+$node->safe_psql(
+	"postgres", qq(
+        SELECT count(1) FROM pg_stat_activity
+		WHERE backend_type = 'worker_spi dynamic';));

This does not check that the worker that does not have the flag set is
still running: you are not feeding the output of this query to an is()
test.

+	is($result, 't', "dynamic bgworker launched");

In launch_bgworker(), this uses the same test description for all the
callers of this subroutine.  Let's prefix it with $testcase.

+void

FWIW, while reading this code, I was wondering about one improvement
that could show benefits for more extension code than only what we are
discussing here because external code has no access to
BackgroundWorkerSlot while holding the LWLock BackgroundWorkerLock in
a single loop, by rewriting this new routine with something like that:
void TerminateBackgroundWorkerMatchin(
bool (*do_terminate) (int pid, BackgroundWorker *, Datum))

Then the per-database termination would be a custom routine, defined
also in bgworker.c.  Other extension code could define their own
filtering callback routine.  Just an idea in passing, to let extension
code take more actions on bgworker slots in use-based on a PGPROC
entry, like a role ID for example, or it could be a different factor.
Feel free to dislike such a funky idea if you do not like it and say
so, of course.
--
Michael

---

From: "Aya Iwata (Fujitsu)" <[email-masked]>
Date: Tue, 21 Oct 2025 14:14:34 +0000

Hi Peter-san, Michael-san,

Thank you for your comments.
I updated patch to v0009. Please review attached patch.

> Sent: Monday, October 20, 2025 11:02 AM

> just "oid". (ditto for the extern in bgworker.h)

I agree with you. I reverted parameter name to databaseId.

>  * Can be reduced to 10 attempts (max 1s total) to speed up tests.

Thank you. I updated this comment.

> 'tries' can be declared as a for-loop variable.

I have declared "int" within the for-loop.

> Can we give this function a better name, or is that out of scope for this patch?

I think this is out of scope because existing code have terminated autovacuum process by SIGTERM.
It can be discussed separately.
I just added a comment to this function "background workers would also be terminated".

> retries, allowing for shorter test runs. See CountOtherDBBackends().

Thank you for your suggestion. I updated this comment.

> Sent: Monday, October 20, 2025 1:33 PM

> after the first test with CREATE DATABASE works here.

Thank you. I updated ntries to 3.

> callers of this subroutine.  Let's prefix it with $testcase.

I added $testcase. Is it same as your expectations?

> Feel free to dislike such a funky idea if you do not like it and say

Thank you for your advice.
I'd like to address that, but I couldn't figure out how to do it on my own. 
Could you please describe it more?

Regards,
Aya Iwata
Fujitsu Limited

---

From: "Aya Iwata (Fujitsu)" <[email-masked]>
Date: Thu, 20 Nov 2025 06:57:47 +0000

Hi

Perhaps you haven't seen my previous post.

> Could you please describe it more?

I'd like to adapt the patch for this, so could you tell me with the details?

Regards,
Aya Iwata
Fujitsu Limited

---

From: Pavel Stehule <[email-masked]>
Date: Sun, 14 Dec 2025 08:40:20 +0100

Hi

> +#define BGWORKER_EXIT_AT_DATABASE_CHANGE       0x0004
I am checking this patch, and I think so used names can be little bit
confusing

BGWORKER_EXIT_AT_DATABASE_CHANGE - it is used for disconnecting workers on
the template database, and this database is not changing.

TerminateBgWorkersByDbOid - it doesn't terminate all workers, but only
workers with some special flags

Maybe BGWORKER_INTERRUPTABLE and TerminateInterruptableBgWorkersByDbOid ?

Another question is if this cancellation should be implicit and should not
require some special flag.

When I want to disconnect connections to database when I do drop, I have to
use FORCE flag

So maybe there should be ALTER DATABASE ... RENAME ... FORCE - or if FORCE
can terminare all workers (without special FLAG) ?

Regards

Pavel

---

From: "Aya Iwata (Fujitsu)" <[email-masked]>
Date: Mon, 15 Dec 2025 12:56:22 +0000

Hi

Thank you for your review.

> From: Pavel Stehule <[email-masked]> 
> Sent: Sunday, December 14, 2025 4:40 PM
> To: Michael Paquier <[email-masked]>
> Cc: Iwata, Aya/岩田 彩 <[email-masked]>; Peter Smith <[email-masked]>; Chao Li <[email-masked]>; Kuroda, Hayato/黒田 隼人 <[email-masked]>; pgsql-hackers <[email-masked]>
> Subject: Re: [PROPOSAL] Termination of Background Workers for ALTER/DROP DATABASE
> 
> I am checking this patch, and I think so used names can be little bit confusing
> 
> BGWORKER_EXIT_AT_DATABASE_CHANGE - it is used for disconnecting workers on the template database, and this database is not changing.
> 
> TerminateBgWorkersByDbOid - it doesn't terminate all workers, but only workers with some special flags
> 

Thank you for your advice.
I changed the name of a function and a flag. 

> Another question is if this cancellation should be implicit and should not require some special flag.
> 
> When I want to disconnect connections to database when I do drop, I have to use FORCE flag
> 
> So maybe there should be ALTER DATABASE ... RENAME ... FORCE - or if FORCE can terminare all workers (without special FLAG) ?

For the proposed feature, we've added a flag allowing each extension developer to decide whether to terminate it via DROP/ALTER DATABASE.
Adding a FORCE option to ALTER to let database definition modifiers decide whether to force termination of background workers might be better discussed in a separate thread.

Best Regards,
Aya Iwata

---

From: Pavel Stehule <[email-masked]>
Date: Mon, 15 Dec 2025 14:18:19 +0100

po 15. 12. 2025 v 13:56 odesílatel Aya Iwata (Fujitsu) <
[email-masked]> napsal:

> > Cc: Iwata, Aya/岩田 彩 <[email-masked]>; Peter Smith <
> [email-masked]>; Chao Li <[email-masked]>; Kuroda,
> Hayato/黒田 隼人 <[email-masked]>; pgsql-hackers <
> [email-masked]>
> > BGWORKER_EXIT_AT_DATABASE_CHANGE - it is used for disconnecting workers
> on the template database, and this database is not changing.
> >
> > So maybe there should be ALTER DATABASE ... RENAME ... FORCE - or if
> FORCE can terminare all workers (without special FLAG) ?
>
> For the proposed feature, we've added a flag allowing each extension
> developer to decide whether to terminate it via DROP/ALTER DATABASE.
> Adding a FORCE option to ALTER to let database definition modifiers decide
> whether to force termination of background workers might be better
> discussed in a separate thread.
>

When I thought about it - there can be a second alternative.

Introduce a pair of flags BGWORKER_INTERRUPTABLE and BGWORKER_PROTECTED
(the names can be enhanced or changed). BGWORKER_INTERRUPTABLE can be
default.
ALTER DATABASE RENAME and related commands can stop any non protected
workers. ALTER DATABASE RENAME FORCE can stop any workers (including
protected).

Is there any reason why BGWORKER_INTERRUPTABLE cannot be default? Probably
nobody would block some possibly common operations on database level
without strong reason.

Regards

Pavel

---

From: "Aya Iwata (Fujitsu)" <[email-masked]>
Date: Wed, 17 Dec 2025 13:31:51 +0000

Hi Pavel-san,

> Introduce a pair of flags BGWORKER_INTERRUPTABLE and BGWORKER_PROTECTED (the names can be enhanced or changed). BGWORKER_INTERRUPTABLE can be default. 
> ALTER DATABASE RENAME and related commands can stop any non protected workers. ALTER DATABASE RENAME FORCE can stop any workers (including protected). 

I can't image any use cases for BGWORKER_PROTECTED. Do you have any idea?
Also, I think the parameter settings might get a complicated.
If we start discussing the "FORCE" option, it is better to think about this parameter.

> Is there any reason why BGWORKER_INTERRUPTABLE cannot be default? Probably nobody would block some possibly common operations on database level without strong reason.

As Michael-san mentioned in a previous email, this behavior has remained unchanged since bgworkers were introduced in v9.3. 
I don't see a compelling reason to alter it now.  Additionally, this specification can be modified later.

Best Regards,
Aya Iwata

---

From: Pavel Stehule <[email-masked]>
Date: Wed, 17 Dec 2025 16:33:16 +0100

Hi

st 17. 12. 2025 v 14:31 odesílatel Aya Iwata (Fujitsu) <
[email-masked]> napsal:

> > Adding a FORCE option to ALTER to let database definition modifiers
> decide whether to force termination of background workers might be better
> > Is there any reason why BGWORKER_INTERRUPTABLE cannot be default?
> Probably nobody would block some possibly common operations on database
> level without strong reason.
>
> As Michael-san mentioned in a previous email, this behavior has remained
> unchanged since bgworkers were introduced in v9.3.

I understand the request for unchanging behaviour - but I am not sure if
this concept is really helpful - or if the naming is best. I am afraid so
this feature without changing the workers code is useless (and maybe it is
wanted).

Any worker should be interruptable by sigterm. And then the
name BGWORKER_INTERRUPTABLE is little bit vague. Maybe some
like BGWORKER_CAREFREE_INTERRUPTABLE can be better (or some like this -
maybe BGWORKER_CANCELABLE)? This can be a signal from bgworker's authors -
it is ok to kill the worker anytime when it is necessary.

Some workers can have the flag BGW_NEVER_RESTART - cannot be used as signal
so this worker is protected, and others can be terminated safely, because
they will be restarted after 60 seconds?

Regards

Pavel

---

From: "Aya Iwata (Fujitsu)" <[email-masked]>
Date: Thu, 18 Dec 2025 11:47:31 +0000

Hi Pavel-san

Thank you for your feedback!

> Sent: Thursday, December 18, 2025 12:33 AM
> Cc: Michael Paquier <[email-masked]>; Peter Smith <[email-masked]>; Chao Li <[email-masked]>; Kuroda, Hayato/黒田 隼人 <[email-masked]>; pgsql-hackers <[email-masked]>

> I understand the request for unchanging behaviour - but I am not sure if this concept is really helpful - or if the naming is best. I am afraid so this feature without changing the workers code is useless (and maybe it is wanted).

It is our intention; this feature would enable when developer expressly set.

> Any worker should be interruptable by sigterm. And then the name BGWORKER_INTERRUPTABLE is little bit vague. Maybe some like BGWORKER_CAREFREE_INTERRUPTABLE can be better (or some like this - maybe BGWORKER_CANCELABLE)? This can be a signal from bgworker's authors - it is ok to kill the worker anytime when it is necessary. 

That's right, "interruptable" may not be appropriate. This is because even bgworkers without this flag set can be interrupted by sigterm.
Hmm, I feel these ideas may not be clear what it does. Do someone have other idea?

> Some workers can have the flag BGW_NEVER_RESTART - cannot be used as signal so this worker is protected, and others can be terminated safely, because they will be restarted after 60 seconds?

In my understanding, you are suggesting that the bgworker which set bgw_restart_time as BGW_NEVER_RESTART is not terminated by DROP/ALTER, right? 
Do you think what kind of use cases might there be?

Best Regards,
Aya Iwata

---

From: Pavel Stehule <[email-masked]>
Date: Thu, 18 Dec 2025 17:04:50 +0100

čt 18. 12. 2025 v 12:47 odesílatel Aya Iwata (Fujitsu) <
[email-masked]> napsal:

> > Cc: Michael Paquier <[email-masked]>; Peter Smith <
> >> > Introduce a pair of flags BGWORKER_INTERRUPTABLE and
> BGWORKER_PROTECTED (the names can be enhanced or changed).
> BGWORKER_INTERRUPTABLE can be default.
> >> As Michael-san mentioned in a previous email, this behavior has
> remained unchanged since bgworkers were introduced in v9.3.
> > Any worker should be interruptable by sigterm. And then the name
> BGWORKER_INTERRUPTABLE is little bit vague. Maybe some like
> BGWORKER_CAREFREE_INTERRUPTABLE can be better (or some like this - maybe
> BGWORKER_CANCELABLE)? This can be a signal from bgworker's authors - it is
> ok to kill the worker anytime when it is necessary.
>
> That's right, "interruptable" may not be appropriate. This is because even
> bgworkers without this flag set can be interrupted by sigterm.
> > Some workers can have the flag BGW_NEVER_RESTART - cannot be used as
> signal so this worker is protected, and others can be terminated safely,
> because they will be restarted after 60 seconds?
>
> In my understanding, you are suggesting that the bgworker which set
> bgw_restart_time as BGW_NEVER_RESTART is not terminated by DROP/ALTER,
> right?

I don't think about use cases - just workers that can be restarted can be
terminated with less  risk than workers that shouldn't be restarted - and
maybe BGWORKER_CAREFREE_INTERRUPTABLE is just negation of BGW_NEVER_RESTART
in almost cases.

I afraid so BGWORKER_CAREFREE_INTERRUPTABLE (or
maybe BGWORKER_SAFELY_INTERRUPTABLE) will be source of bugs - this flag is
just "safeguard" for very special cases - and very common bug will be
omission of this flug until someone reports (after long time) "it block
database rename".

Second objection against this design is inconsistency with current CREATE,
DROP DATABASE commands. Without the FORCE flag, it terminates nothing. And
with the FORCE flag it terminates anything that is necessary. Proposed
design is safe - because only specially marked bgworkers are terminated,
but for users it can be unwanted surprise still - and at the end in next
versions - mostly bgworkers will have this flag, because nobody want to
block DDL

Regards

Pavel

---

From: "Aya Iwata (Fujitsu)" <[email-masked]>
Date: Wed, 24 Dec 2025 08:42:51 +0000

Hi

> Feel free to dislike such a funky idea if you do not like it and say

I'm sorry for the delayed response.
I tried implementing a callback function within the for loop to allow setting the termination
condition myself. However, I felt this method had few use-cases. Since LWlock exists in here, it can
only handle lightweight operations.
Therefore, I'd prefer not to include it at this time. If I later think of a useful scenario for it, I'll 
reconsider about this.

Best Regards,
Aya Iwata

---

From: "Aya Iwata (Fujitsu)" <[email-masked]>
Date: Thu, 25 Dec 2025 13:24:33 +0000

Hi

https://www.postgresql.org/message-id/aPBoXXW3XuwiIsHG%40paquier.xyz

Based on previous discussions, we had configured DROP/ALTER DATABASE to
terminate only background workers with specific parameters set.

To reconsider the default behavior, I created the following patches:
0001 is the patch for the existing implementation.
0002 is the patch modified based on Pavel-san's suggestion.

The 0002 patch changes the default behavior to terminate background workers
when DROP/ALTER DATABASE is executed. It also includes a test set
for this change.
For background workers that should not be terminated, setting
the BGWORKER_PROTECTED parameter prevents this.

The relationship with the FORCE option during DROP DATABASE is as follows:

FORCE option present, flag present... TERMINATE
FORCE option present, flag absent... TERMINATE
FORCE option absent, flag present... Does not TERMINATE due to the flag
FORCE option absent, flag absent... TERMINATE

I would like to know your comments on which implementation is preferable.

Regards,
Aya Iwata

---

From: "Ryo Matsumura (Fujitsu)" <[email-masked]>
Date: Fri, 26 Dec 2025 10:17:27 +0000

Hi Pavel-san, Iwata-san,

+1 to Allow-background-workers-to-be-terminated

The result is same, so I think it's better to prioritize compatibility.

PGWORKER_PROTECTED would be used in scenarios like the following:
Existing features are probably not designed to be forcibly stopped.
Therefore, all existing features should have PROTECTED applied to them.
Most newly implemented features will also have PROTECTED applied because it requires less thought and is safer.
Only considerate developers of features that can easily guarantee safety would adopt the default.

In conclusion, this is no different from BGWORKER_INTERRUPTABLE.
Therefore, I think it's better to prioritize compatibility.

Best Regards
Ryo Matsumura

---

From: Pavel Stehule <[email-masked]>
Date: Fri, 26 Dec 2025 12:04:47 +0100

Hi

> Therefore, I think it's better to prioritize compatibility.

I am not sure if I understand what you prefer.

I share your opinion that most developers use options that are more safe
and require less thinking (and if this option will not be default, most
developers maybe don't use it in the first cycle).

At the end most bgworkers will be not be marked as interruptible (isn't
important if we use flag INTERRUPTIBLE or PROTECTED). Then proposed flag
without FORCE clause will be mostly inefficient - and then there is strong
question if proposed feature has some real benefit. I don't think so this
feature should be implemented and committed once, but we should to find a
consensus on implemented behaviour in all possible cases, and all possible
question.

1. When bgworker is marked as INTERRUPTIBLE, then can be cancelled
immediately - there is full agreement - I don't see any problem - just
probably almost all bgworkers will be not be marked as INTERRUPTIBLE

2.  When bgworker is not marked as INTERRUPTIBLE, then can it be cancelled?
How dangerous is this? If it cannot be cancelled, then ALTER should wait on
lock forever, or there should be some special timeout - like CREATE, DROP
DATABASE and waiting on template databases.

3. If the user needs to execute ALTER, then probably he manually terminates
the worker any time. Are currently used workers safe against terminating?
We know so mostly workers will be restarted after timeout - and then it is
"safe".  It is really safe? Can we expect it? If it is safe, then we can
implement FORCE clause. Probably any worker can be restarted - and without
safety it is hard to imagine using in the production.

I think there is a fundamental question that should be solved before - what
is a risk of canceling bgworker? The possible reply is so there is not any
risk for correctly implemented bgworkers. And then there is a question if
we still need the flag INTERRUPTIBLE? My opinion for this case is probably
yes, because cancelling can introduce some bigger latency.

Regards

Pavel

---

From: Michael Paquier <[email-masked]>
Date: Mon, 29 Dec 2025 09:52:07 +0900

> I am not sure if I understand what you prefer.

Seems to me that Iwata-san sides with compatibility.  Spoiler alert: I
do side with compatibility.  See below for more details.

> probably almost all bgworkers will be not be marked as INTERRUPTIBLE

Yes, these can and should be stopped.

> DATABASE and waiting on template databases.

Cancellation is a different thing.  bgworkers can be tweaked to answer
to specific signals with their own handlers.  What we are discussing
here is if a signal should be sent to a bgworker or not when
performing specific operations.

> safety it is hard to imagine using in the production.

That would be up to an extension developer.  The new behavior can be
useful in some cases.  Forcing it by default could lead to unwanted
results.

> yes, because cancelling can introduce some bigger latency.

I can think about two reasons at the top of my mind, at least:
1) Making the background workers non-interruptible is the default
behavior that we have since 9.3.  Changing the default by allowin
these to be interrupted could lead to a silent behavior change that
extension developers are not aware of because the bgworker lubrary
code would still be compiled as-is, and would still be able to start
correctly.  This links to my second point.
2) Operator error, and these tend to happen a lot.  Somebody with the
right to create a database could decide to use as template a database
that a bgworker is linked to, leading to failures with operations
background workers expect to be able to achieve while online if we
change the default, because it does things that are critical and
should not be interrupted (one could compare that to what an
autovacuum worker does with an antiwraparound work, perhaps, which
cannot be interrupted).

As a whole, I think that we have more advantages in keeping the
default, making the possibility to make a bgworker interruptible an
opt-in choice is extra cream that can be added on top of a cake, and
one is free to add the extra cream to their portion of the cake.
--
Michael

---

From: Michael Paquier <[email-masked]>
Date: Mon, 29 Dec 2025 11:03:39 +0900

> Only considerate developers of features that can easily guarantee safety would adopt the default.

We could design things so as we have a second flag to force a bgworker
to be non-interuptible, then we could force that either the
interruptible flag or the non-interruptible flag should be set.  What
is mentioned as a problem is that 0 implies that the non-interruptible
is enforced.  I don't think that we would have much to gain by doing
that, as it would just lead to extension breakages that we can avoid.

> Therefore, I think it's better to prioritize compatibility.

Looking finally at the patch, I like the simplicity of what you are
doing here.

+      Requires both <literal>BGWORKER_SHMEM_ACCESS</literal> and
+      <literal>BGWORKER_BACKEND_DATABASE_CONNECTION</literal>.

SanityCheckBackgroundWorker() does not enforce this check when
registering a bgworker.  But shouldn't we do so when the interruptible
flag is set?

+void
+TerminateInterruptableBgWorkersByDbOid(Oid databaseId)

Fine by me to aim for simplicity with this interface, discarding my
previous fancy comment about the extensibility we could do here.
Matsumura-san and I have also discussed a bit that offline at the last
JPUG, where I said that I'm OK with simpler at the end.

One issue with the test as written, as of run_db_command(), is that we
make sure that a worker is stopped by scanning the output of the logs.
This approach may detect incorrect patterns, unfortunately.  For
example, if the termination logic has a bug it may be possible that
the worker found as terminated is the first one created by the test,
which we expect to always run.  While the log is mandatory to have, I
have a suggestion to make that even better: let's keep track in
run_db_command() of the PIDs of the worker processes we expect to
exist after running each database command, then make sure that the
list of PIDs match with what we expect.  This is a bit simpler in the
case of this test as we only expect one matching PID.
--
Michael

---

From: Pavel Stehule <[email-masked]>
Date: Mon, 29 Dec 2025 06:31:11 +0100

Hi

> one is free to add the extra cream to their portion of the cake.

ok

Regards

Pavel

---

From: "Aya Iwata (Fujitsu)" <[email-masked]>
Date: Mon, 5 Jan 2026 15:06:53 +0000

Hi 

Thank you for your comments and suggestions on the previous version patches!	
I believe this feature become much better.

I've created a v13 patch incorporating Peter-san's suggestions.

> case of this test as we only expect one matching PID.

I've also reviewed the test set code comparing PIDs. I think this is acceptable.

Best Regards,
Aya Iwata

---

From: Peter Smith <[email-masked]>
Date: Tue, 6 Jan 2026 08:18:39 +1100

> I've created a v13 patch incorporating Peter-san's suggestions.

Some review comments for v13-0001.

======

CountOtherDBBackends:

1.
  * If there are other backends in the DB, we will wait a maximum of 5 seconds
- * for them to exit.  Autovacuum backends are encouraged to exit early by
- * sending them SIGTERM, but normal user backends are just waited for.
+ * for them to exit.  Autovacuum backends and background workers are encouraged
+ * to exit early by sending them PMSIGNAL_BACKGROUND_WORKER_CHANGE, but normal
+ * user backends are just waited for.

This did not seem like the correct fix for my previous review comment
[1, comment #3], because the autovacuum backends are still killed with
SIGTERM, right?

======
.../worker_spi/t/002_worker_terminate.pl

2.
+# Confirm that the non-interruptible bgworker is still running.
+my $result = $node->safe_psql(
+ "postgres", qq(
+        SELECT count(1) FROM pg_stat_activity
+ WHERE backend_type = 'worker_spi dynamic';));

The indentation of the "SELECT" still does not look correct to me. Did
you run pgperltidy on this file?

======
[1] https://www.postgresql.org/message-id/CAHut%2BPtSVYKU4vfaRev4FMdbeZ3ukvxRy4X7uK05jv_9WMYafA%40mail.gmail.com

Kind Regards,
Peter Smith.
Fujitsu Australia

---

From: Michael Paquier <[email-masked]>
Date: Tue, 6 Jan 2026 14:38:06 +0900

> [1, comment #3], because the autovacuum backends are still killed with

Yes, you are right that this creates a confusing mix.  Bgworkers have
no relationship with SIGTERM in this code.  I have added a sentence to
document the new behavior for interruptible bgworkers, after the
existing ones.  One thing that was also missing is the fact that the
wait can be shortened with the injection point.

> you run pgperltidy on this file?

perltidy is happy with that, because it considers the string within
the quoted qq() area as something fine.  At the end this is just a tab
vs whitespace issue, you are right that this should use whitespaces.

I have done a couple of test runs in the CI to be sure, and noticed no
instability in the tests, so applied.  I'll keep an eye on the CFbot
and the buildfarm for the next few days.
--
Michael

---

From: "Aya Iwata (Fujitsu)" <[email-masked]>
Date: Fri, 13 Mar 2026 11:38:51 +0000

Hi

On March 10th, the bgworker test "t/002_worker_terminate.pl" failed on the Build Farm.
https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=jay&dt=2026-03-10%2019%3A26%3A19

We are currently attempting to reproduce this issue and are considering a fix.

According to the log, the test failed because the bgworker cannot be terminated
within the time frame (5 seconds) when ALTER DATABASE RENAME is executed.

So I suspect that there is a lock conflict.

Log of Build Farm;
2026-03-10 21:08:33.103 CET [28076:4] 002_worker_terminate.pl LOG:  statement: ALTER DATABASE testdb RENAME TO renameddb
2026-03-10 21:08:34.578 CET [28071:2] FATAL:  terminating background worker "worker_spi dynamic" due to administrator command
2026-03-10 21:08:38.109 CET [28076:5] 002_worker_terminate.pl ERROR:  database "testdb" is being accessed by other users
2026-03-10 21:08:38.109 CET [28076:6] 002_worker_terminate.pl DETAIL:  There is 1 other session using the database.
2026-03-10 21:08:38.109 CET [28076:7] 002_worker_terminate.pl STATEMENT:  ALTER DATABASE testdb RENAME TO renameddb
2026-03-10 21:08:38.229 CET [28076:8] 002_worker_terminate.pl LOG:  disconnection: session time: 0:00:05.129 user=buildfarm database=postgres host=[local]
2026-03-10 21:08:38.231 CET [28025:6] LOG:  background worker "worker_spi dynamic" (PID 28071) exited with exit code 1
2026-03-10 21:08:38.234 CET [28025:7] LOG:  received immediate shutdown request
2026-03-10 21:08:38.241 CET [28025:8] LOG:  database system is shut down

If there is a lock conflict, we will come up with a solution.

Regards,
Aya Iwata

---

From: Michael Paquier <[email-masked]>
Date: Mon, 16 Mar 2026 16:23:40 +0900

> https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=jay&dt=2026-03-10%2019%3A26%3A19

Indeed.

> So I suspect that there is a lock conflict.

Aye, we have a timing issue here, and at first it seemed to me that
this was a bug in the backend logic.  But actually I suspect that it
is simpler than that: we don't disable autovacuum so couldn't an
autovacuum worker connect to the database "testdb" that we are trying
to rename in this query?  I cannot be 100% sure because we have
reduced the log activity for the sake of the tests, but that feels
possible to me.  We could try to disable autovacuum entirely, then see
if the situation gets better in the buildfarm.

The failure rate is so low that it is likely going to take a few weeks
to check the stability of the situation.  We could also lot more
things, of course, as a temporary solution.

What do you think?
--
Michael

---

From: Michael Paquier <[email-masked]>
Date: Tue, 17 Mar 2026 08:37:39 +0900

> things, of course, as a temporary solution.

After sleeping on it, I do not have a better idea than the attached,
so we could always get that into the tree and see if the situation
improves, even if it would take time.  Iwata-san and others, what do
you think?  Perhaps you could think of a different reason causing this
failure?
--
Michael
diff --git a/src/test/modules/worker_spi/t/002_worker_terminate.pl b/src/test/modules/worker_spi/t/002_worker_terminate.pl
index 6d3794355981..21dfb5961289 100644
--- a/src/test/modules/worker_spi/t/002_worker_terminate.pl
+++ b/src/test/modules/worker_spi/t/002_worker_terminate.pl
@@ -59,6 +59,10 @@ sub run_bgworker_interruptible_test
 
 my $node = PostgreSQL::Test::Cluster->new('mynode');
 $node->init;
+$node->append_conf(
+	"postgresql.conf", qq(
+autovacuum = off
+));
 $node->start;
 
 # Check if the extension injection_points is available, as it may be

---

From: Kuroda, Hayato/黒田 隼人<[email-masked]>
Date: Tue, 17 Mar 2026 01:55:05 +0000

Dear Michael,

> you think?  Perhaps you could think of a different reason causing this

I also had a discussion with her off-list, but I could not find neither.
Additional idea is to increase the log level to make analysis easier, but not
sure what value is appropriate. Is it enough to raise to DEBUG1?

Best regards,
Hayato Kuroda
FUJITSU LIMITED

---

From: Michael Paquier <[email-masked]>
Date: Tue, 17 Mar 2026 12:57:59 +0900

> sure what value is appropriate. Is it enough to raise to DEBUG1?

That would be an option.  Let's do that if we see another failure
after disabling autovacuum.  Another thing I have noticed is
debug_parallel_query=regress for this animal, which may encourage
parallel workers, so I have also disabled it.  Let's see first if we
get more failures after af8837a10bc7.
--
Michael

---

From: Alexander Lakhin <[email-masked]>
Date: Tue, 17 Mar 2026 08:00:00 +0200

Hello Michael and Iwata-san,

> get more failures after af8837a10bc7.

I've noticed another failure of the test from widowbird: [1]. But probably
it's not related to f1e251be8 directly, as widowbird failed with a similar
error before: [2]. I couldn't reproduce such failures yet, but keep
trying in the background...

[1] https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=widowbird&dt=2026-03-16%2009%3A35%3A03
[2] https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=widowbird&dt=2025-10-25%2010%3A35%3A03

Best regards,
Alexander

---

From: Michael Paquier <[email-masked]>
Date: Tue, 17 Mar 2026 17:33:25 +0900

> [2] https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=widowbird&dt=2025-10-25%2010%3A35%3A03

Thanks.  widowbird also uses "debug_parallel_query = regress", so it
really looks like to me that this is related to a parallel worker
spawning and messing with the database being renamed.
--
Michael

---

From: Michael Paquier <[email-masked]>
Date: Thu, 19 Mar 2026 09:54:04 +0900

On Wed, Mar 18, 2026 at 03:52:02PM -0400, Tom Lane wrote:
> which makes me wonder whether the problematic session is the second or
> third bgworker.  I am not seeing entries indicating that those
> stopped, as there is for the first bgworker.

Looking at the logs produced at [1], the worker launched as number 1
would not be able to interact, it connects to the database postgres,
under PID 1616001, and is reported as exited by the postmaster.

The only interacting sessions would be:
1) The bgworker launched as number 2, connected to database testdb.
2) The session checking for pg_stat_activity, launched by
launch_bgworker().  The test was connected with the database we want
to rename, and this could interact as an extra session.  This query
could be run while connected to the database postgres to reduce the
friction and discarding this one.

The timestamps of the logs tell that it takes 5 seconds for this host
to get out of the ALTER DATABASE .. RENAME TO, which implies that we
are looping inside CountOtherDBBackends() for 5 seconds.  So it really
looks like the second bgworker is the one we are waiting for here.
Now, we are sure of the following things when we try to launch the
RENAME TO:
- The worker is seen in pg_stat_activity.
- The worker is already in worker_spi_main(), per its "LOG initialized
with" entry.
- The worker is connected to the database.
- The worker can receive signals.

How would it be possible for this worker to not receive the requests?
The only thing I could think of is that the postmaster does not have
the time to process the PMSIGNAL_BACKGROUND_WORKER_CHANGE requests?

The next thing would be to gather more data, I guess.  The attached
would help in providing more information.  If it happens that we are
able to send the requests and that the postmaster does not have the
time to process them, I don't really see what we can do except:
- Drop the portion of the tests for DROP DATABASE, SET TABLESPACE and
RENAME DB, because all these scenarios involve commands that work on
the same database as the worker connected, and if the postmaster does
not have the time to process the termination requests, I don't really
see what we could do.  This could also point to a timing issue with
the feature in itself, of course.
- Revert the feature, stop playing with the buildfarm due to the end
of the release cycle, and rework it for v20.

For now I am planning for the attached to get more information from
widowbird, which should take a few days at worst.  That would make
clear if we have a timing issue with the requests sent to the
postmaster.  Launching the queries for worker_spi_launch() and
pg_stat_activity on the database postgres may also improve things, but
I don't really buy it, even if I may be wrong.

[1]: https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=widowbird&dt=2026-03-17%2015%3A35%3A03
--
Michael
From adb474866f12a66ac48c702d9ab25ce82277abca Mon Sep 17 00:00:00 2001
From: Michael Paquier <[email-masked]>
Date: Thu, 19 Mar 2026 09:51:16 +0900
Subject: [PATCH] Add more debugging information for termination tests of
 worker_spi

---
 src/backend/postmaster/bgworker.c                     |  6 ++++++
 src/test/modules/worker_spi/t/002_worker_terminate.pl | 10 ++++++++--
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/src/backend/postmaster/bgworker.c b/src/backend/postmaster/bgworker.c
index 0104a86b9ecd..fd678ef2596d 100644
--- a/src/backend/postmaster/bgworker.c
+++ b/src/backend/postmaster/bgworker.c
@@ -1413,6 +1413,9 @@ TerminateBackgroundWorkersForDatabase(Oid databaseId)
 {
 	bool		signal_postmaster = false;
 
+	elog(DEBUG1, "attempting worker termination for database %u",
+		 databaseId);
+
 	LWLockAcquire(BackgroundWorkerLock, LW_EXCLUSIVE);
 
 	/*
@@ -1432,6 +1435,9 @@ TerminateBackgroundWorkersForDatabase(Oid databaseId)
 			{
 				slot->terminate = true;
 				signal_postmaster = true;
+
+				elog(DEBUG1, "termination requested for worker (PID %d) on database %u",
+					 slot->pid, databaseId);
 			}
 		}
 	}
diff --git a/src/test/modules/worker_spi/t/002_worker_terminate.pl b/src/test/modules/worker_spi/t/002_worker_terminate.pl
index 6db80ffec88c..b0e6a5376d4c 100644
--- a/src/test/modules/worker_spi/t/002_worker_terminate.pl
+++ b/src/test/modules/worker_spi/t/002_worker_terminate.pl
@@ -24,7 +24,7 @@ sub launch_bgworker
 
 	# Launch a background worker on the given database.
 	my $pid = $node->safe_psql(
-		$database, qq(
+		'postgres', qq(
         SELECT worker_spi_launch($testcase, '$database'::regdatabase, 0, '{}', $interruptible);
     ));
 
@@ -32,7 +32,7 @@ sub launch_bgworker
 	$node->wait_for_log(
 		qr/LOG: .*worker_spi dynamic worker $testcase initialized with .*\..*/,
 		$offset);
-	my $result = $node->safe_psql($database,
+	my $result = $node->safe_psql('postgres',
 		"SELECT count(*) > 0 FROM pg_stat_activity WHERE pid = $pid;");
 	is($result, 't', "dynamic bgworker $testcase launched");
 
@@ -52,6 +52,11 @@ sub run_bgworker_interruptible_test
 		qr/terminating background worker \"worker_spi dynamic\" due to administrator command/,
 		$offset);
 
+	# Postmaster entry reporting the worker as exiting.
+	$node->wait_for_log(
+		qr/LOG: .*background worker \"worker_spi dynamic\" \(PID $pid\) exited with exit code/,
+		$offset);
+
 	my $result = $node->safe_psql('postgres',
 		"SELECT count(*) = 0 FROM pg_stat_activity WHERE pid = $pid;");
 	is($result, 't', "dynamic bgworker stopped for $testname");
@@ -63,6 +68,7 @@ $node->append_conf(
 	"postgresql.conf", qq(
 autovacuum = off
 debug_parallel_query = off
+log_min_messages = debug1
 ));
 $node->start;
 
-- 
2.53.0

---

From: Michael Paquier <[email-masked]>
Date: Sat, 21 Mar 2026 11:46:23 +0900

> I don't really buy it, even if I may be wrong.

A couple of days later after 79a5911fe65b, widowbird seems to have
cooled down a bit:
https://buildfarm.postgresql.org/cgi-bin/show_history.pl?nm=widowbird&br=master

The extra debugging information is proving to be useful.  For example,
looking at this one for the RENAME TO case:
https://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=widowbird&dt=2026-03-19%2003%3A35%3A03&stg=module-worker_spi-check

And some problematic pattern logs:
2026-03-19 04:49:07.848 UTC [271557:4] 002_worker_terminate.pl LOG:
statement: ALTER DATABASE testdb RENAME TO renameddb  
2026-03-19 04:49:07.849 UTC [271557:5] 002_worker_terminate.pl DEBUG:
attempting worker termination for database 16413  
2026-03-19 04:49:07.849 UTC [271557:6] 002_worker_terminate.pl DEBUG:
termination requested for worker (PID 271553) on database 16413
[...]
2026-03-19 04:49:08.732 UTC [271557:19] 002_worker_terminate.pl DEBUG:
attempting worker termination for database 16413  
2026-03-19 04:49:08.732 UTC [271557:20] 002_worker_terminate.pl DEBUG:
termination requested for worker (PID 271553) on database 16413  

So we are able to send the requests to the workers, and these can take
a long time before being processed by the postmaster.  Querying
directly "postgres" for the worker_spi_launch() and pg_stat_activity
queries seems to have reduced the friction, with less requests to
send.   However, I don't think that this is the end of the story, even
after 79a5911fe65b I have spotted one case of RENAME TO where the
requests were sent for a bit more than 4s, before the postmaster had
the idea to catch up.  RENAME TO is the only one that can get slow
(really no idea why), so I guess that we could always tweak things a
bit more:
1) Extra injection point to increase the timeout (30s or 60s?) and
give the postmaster more room to proceed the requests.
2) Remove this portion of the test, but it would be sad.

I'll keep an eye for more failures, even if the situation is looking
slightly better.
--
Michael

---

From: Alexander Lakhin <[email-masked]>
Date: Tue, 31 Mar 2026 10:00:00 +0300

Hello Michael,

> I'll keep an eye for more failures, even if the situation is looking

Having reproduced this locally (running 3 tests in parallel with
ALTER DATABASE RENAME repeated 200 times, on a slow riscv64 machine), I
discovered that in the bad case the worker doesn't reach the main loop in
time (and CHECK_FOR_INTERRUPTS() inside it), because it doesn't get out of
initialize_worker_spi() -> CommitTransactionCommand().

With this modification:
--- a/src/backend/storage/ipc/procarray.c
+++ b/src/backend/storage/ipc/procarray.c
@@ -3752,3 +3752,3 @@ CountOtherDBBackends(Oid databaseId, int *nbackends, int *nprepared)
          */
-       int                     ntries = 50;
+       int                     ntries = 500;

@@ -3798,3 +3798,6 @@ CountOtherDBBackends(Oid databaseId, int *nbackends, int *nprepared)
                 if (!found)
+{
+elog(LOG, "!!!CountOtherDBBackends| found no backends, try %d", tries);
                         return false;           /* no conflicting backends, so done */
+}

I can see the following:
... !!!CountOtherDBBackends| found no backends, try 1
# most of the calls (200 of 201) succeeded with try 1, but there are also:
... !!!CountOtherDBBackends| found no backends, try 7
... !!!CountOtherDBBackends| found no backends, try 51
... !!!CountOtherDBBackends| found no backends, try 74
... !!!CountOtherDBBackends| found no backends, try 84

So the backend is not completely stuck, but CommitTransactionCommand()
may take more than 5 seconds under some circumstances (maybe it's worth
investigating which exactly).

Best regards,
Alexander

---

From: Michael Paquier <[email-masked]>
Date: Tue, 31 Mar 2026 19:54:30 +0900

> investigating which exactly).

One could blame slow hardware, difficult to say, and I'm puzzled by
these periodic bumps that don't seem to happen elsewhere.  There is at
least one alternative that I can think here to make the test more
stable and make sure that worker_spi reaches its main loop before
attempting the database commands.  We could add a wait_for_event() at
the end of launch_bgworker() based on WorkerSpiMain, and enlarge
worker_spi_naptime to an insanely larger value, to make sure that we
remain on the WaitLatch() until the worker is interrupted and that we
don't attempt a new transaction.
--
Michael

---

From: Alexander Lakhin <[email-masked]>
Date: Tue, 31 Mar 2026 20:00:00 +0300

31.03.2026 13:54, Michael Paquier wrote:
> these periodic bumps that don't seem to happen elsewhere.

I managed to get the backtrace of such a sluggish backend:
Using host libthread_db library "/lib/riscv64-linux-gnu/libthread_db.so.1".
0x0000003fb1f4cc26 in posix_fadvise64 () from /lib/riscv64-linux-gnu/libc.so.6
   Id   Target Id                                   Frame
* 1    Thread 0x3fb2a4c620 (LWP 564194) "postgres" 0x0000003fb1f4cc26 in posix_fadvise64 () from 
/lib/riscv64-linux-gnu/libc.so.6
#0  0x0000003fb1f4cc26 in posix_fadvise64 () from /lib/riscv64-linux-gnu/libc.so.6
#1  0x0000002abef79444 in XLogFileClose () at xlog.c:3672
#2  0x0000002abef7cc66 in XLogWrite (WriteRqst=..., tli=tli@entry=1, flexible=flexible@entry=false) at xlog.c:2356
#3  0x0000002abef7dbfc in XLogFlush (record=33561688) at xlog.c:2892
#4  0x0000002abef77976 in RecordTransactionCommit () at xact.c:1516
#5  CommitTransaction () at xact.c:2379
#6  0x0000002abef78938 in CommitTransactionCommandInternal () at xact.c:3224
#7  0x0000002abef78acc in CommitTransactionCommand () at xact.c:3185
#8  0x0000003fb2a3ed88 in initialize_worker_spi (table=0x2abf8bf358) at worker_spi.c:132
#9  worker_spi_main (main_arg=<optimized out>) at worker_spi.c:181
....
(Three test runs produced the same stack trace.)

I think this can explain slow CommitTransactionCommand() and why it
happens not every time. Regarding other animals, I guess they can
experience the same bumps but not exceeding 5 seconds (50 tries). Thus,
from my understanding, for the failure to happen, we need to have slow
storage and initialize_worker_spi() -> CommitTransactionCommand() reaching
XLogFileClose().

Best regards,
Alexander

---

From: Tom Lane <[email-masked]>
Date: Wed, 01 Apr 2026 16:20:10 -0400

> storage and initialize_worker_spi() -> CommitTransactionCommand() reaching

So, it remains not very clear why only widowbird is showing this
failure, but I think we can safely take away the bottom-line
conclusion that hard-wiring a maximum wait of 5s in
CountOtherDBBackends() was not a great idea.

I don't think I want to propose a GUC for this, but could it make
sense to check for an environment variable, similarly to PGCTLTIMEOUT
and PG_TEST_TIMEOUT_DEFAULT?  Whichever way we do it, it could replace
the existing crude hack to change the max wait in injection-point
mode.

			regards, tom lane

---

From: "Hayato Kuroda (Fujitsu)" <[email-masked]>
Date: Thu, 2 Apr 2026 03:20:59 +0000

Dear hackers,

Thanks for paying attention the thread.
I've been considering why the XLogFileClose() is slow only on widowbird but I
have no idea. But on this thread, I think we can just put a workaround for
stabilization.

> the existing crude hack to change the max wait in injection-point

I feel it might be a better approach, attached one implemented the idea.
I did not use the term "_TEST_" because such env variables are only referred
in test modules or binaries. But I did not update the documentation.

Best regards,
Hayato Kuroda
FUJITSU LIMITED

---

From: Alexander Lakhin <[email-masked]>
Date: Thu, 2 Apr 2026 07:00:00 +0300

Hello Tom and Tomas,

01.04.2026 23:20, Tom Lane wrote:
> Alexander Lakhin<[email-masked]> writes:

There also were two failures from jay: [1], [2], but yes, widowbird is
getting more and more consistent in that aspect: [3], probably because
of the storage (SD card?) degradation.

Tomas, maybe you could check if the write speed is more or less acceptable
there?

Regarding the test-specific fix, let me remind of the other failure from
widowbird: [4].

[1] https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=jay&dt=2026-03-10%2019%3A26%3A19
[2] https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=jay&dt=2026-03-24%2020%3A43%3A24
[3] https://buildfarm.postgresql.org/cgi-bin/show_history.pl?nm=widowbird&br=master
[4] https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=widowbird&dt=2025-10-25%2010%3A35%3A03

Best regards,
Alexander

---

From: "Hayato Kuroda (Fujitsu)" <[email-masked]>
Date: Thu, 2 Apr 2026 06:57:10 +0000

> I feel it might be a better approach, attached one implemented the idea.

Note - the patch just set the env variable to the larger value at the later part,
but of course we can restore the value set on the user.
There we must do is to shorten the test time at the beginning then reset it.

Best regards,
Hayato Kuroda
FUJITSU LIMITED

---

From: Tomas Vondra <[email-masked]>
Date: Thu, 2 Apr 2026 13:26:32 +0200

> of the storage (SD card?) degradation.

Jay is a regular machine, 2-core VM hosted at a university, so not very
powerful but was running for years just fine and seems to be healthy.

> Tomas, maybe you could check if the write speed is more or less acceptable

Will do. I'll wait for the tests to complete on widowbird, and will do
some testing on the storage (it's running from a flash drive, not SD
card, and I saw stuff in dmesg when it was dying in the past - but now
it's clean).

regards

-- 
Tomas Vondra

---

From: Michael Paquier <[email-masked]>
Date: Sun, 5 Apr 2026 20:47:30 +0900

> have no idea. But on this thread, I think we can just put a workaround for

I am not convinced that we have a need for an environment variable for
this case, TBH.  It would be enough to make sure that the spawned
worker is paused on its WaitLatch(), with a naptime large enough to
allow the initial transaction to commit, as in the attached.  When the
interruption is run, the worker would stop at CHECK_FOR_INTERRUPTS(),
before beginning a new transaction.
--
Michael
From fe0a2284d87930cd18d13ff4c473f96de5a965e8 Mon Sep 17 00:00:00 2001
From: Michael Paquier <[email-masked]>
Date: Sun, 5 Apr 2026 20:45:16 +0900
Subject: [PATCH] Improve stability of worker_spi termination test

This switches the test to wait for the spawned bgworkers to reach their
main loops, napping.  worker_spi.naptime is enlarged to 10 minutes to
give enough room for slow machines to reach this state.
---
 .../worker_spi/t/002_worker_terminate.pl        | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/src/test/modules/worker_spi/t/002_worker_terminate.pl b/src/test/modules/worker_spi/t/002_worker_terminate.pl
index b0e6a5376d4c..a7d507cc0596 100644
--- a/src/test/modules/worker_spi/t/002_worker_terminate.pl
+++ b/src/test/modules/worker_spi/t/002_worker_terminate.pl
@@ -20,7 +20,6 @@ if ($ENV{enable_injection_points} ne 'yes')
 sub launch_bgworker
 {
 	my ($node, $database, $testcase, $interruptible) = @_;
-	my $offset = -s $node->logfile;
 
 	# Launch a background worker on the given database.
 	my $pid = $node->safe_psql(
@@ -28,13 +27,11 @@ sub launch_bgworker
         SELECT worker_spi_launch($testcase, '$database'::regdatabase, 0, '{}', $interruptible);
     ));
 
-	# Check that the bgworker is initialized.
-	$node->wait_for_log(
-		qr/LOG: .*worker_spi dynamic worker $testcase initialized with .*\..*/,
-		$offset);
-	my $result = $node->safe_psql('postgres',
-		"SELECT count(*) > 0 FROM pg_stat_activity WHERE pid = $pid;");
-	is($result, 't', "dynamic bgworker $testcase launched");
+	# Check that the bgworker is initialized and napping.
+	my $result = $node->poll_query_until('postgres',
+		qq[SELECT wait_event FROM pg_stat_activity WHERE pid = $pid;],
+		qq[WorkerSpiMain]);
+	is($result, 1, "dynamic bgworker $testcase launched");
 
 	return $pid;
 }
@@ -64,11 +61,15 @@ sub run_bgworker_interruptible_test
 
 my $node = PostgreSQL::Test::Cluster->new('mynode');
 $node->init;
+# The naptime is large enough to give room on slow machines to reach
+# the main loop of the spawned workers, after these initialize their
+# schemas.
 $node->append_conf(
 	"postgresql.conf", qq(
 autovacuum = off
 debug_parallel_query = off
 log_min_messages = debug1
+worker_spi.naptime = 600
 ));
 $node->start;
 
-- 
2.53.0