Re: [PROPOSAL] Termination of Background Workers for ALTER/DROP DATABASE - Mailing list pgsql-hackers
| From | Chao Li |
|---|---|
| Subject | Re: [PROPOSAL] Termination of Background Workers for ALTER/DROP DATABASE |
| Date | |
| Msg-id | E8C35CCE-E92F-467B-A604-5C1BBBA8F1C8@gmail.com Whole thread Raw |
| In response to | RE: [PROPOSAL] Termination of Background Workers for ALTER/DROP DATABASE ("Aya Iwata (Fujitsu)" <iwata.aya@fujitsu.com>) |
| Responses |
RE: [PROPOSAL] Termination of Background Workers for ALTER/DROP DATABASE
|
| List | pgsql-hackers |
Hi Iwata-san,
On Oct 9, 2025, at 21:09, Aya Iwata (Fujitsu) <iwata.aya@fujitsu.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.
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
HighGo Software Co., Ltd.
https://www.highgo.com/
pgsql-hackers by date: