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

Hi Iwata-san,

A few comments:

> On Oct 9, 2025, at 21:09, Aya Iwata (Fujitsu) <iwata(dot)aya(at)fujitsu(dot)com> wrote:
>
> Hi,
>
> I updated the patch to v0005.
>
> Regards,
> Aya Iwata
> Fujitsu Limited
> <v0005-0001-Allow-background-workers-to-be-terminated.patch>

1 - bgworker.sgml
```
+ <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>
```

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
```
+extern void TerminateBackgroundWorkersByOid(Oid databaseId);
```

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
```
+ /*
+ * Terminate all background workers for this database, if
+ * they had requested it (BGWORKER_EXIT_AT_DATABASE_DROP)
+ */
+ TerminateBackgroundWorkersByOid(databaseId);
```

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
```
+/*
+ * Cancel background workers.
+ */
+void
+TerminateBackgroundWorkersByOid(Oid databaseId)
```

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/