public inbox for [email protected]
help / color / mirror / Atom feedFrom: Peter Smith <[email protected]>
To: Aya Iwata (Fujitsu) <[email protected]>
Cc: Chao Li <[email protected]>
Cc: Hayato Kuroda (Fujitsu) <[email protected]>
Cc: Michael Paquier <[email protected]>
Cc: pgsql-hackers <[email protected]>
Subject: Re: [PROPOSAL] Termination of Background Workers for ALTER/DROP DATABASE
Date: Mon, 13 Oct 2025 09:28:40 +1100
Message-ID: <CAHut+PtbOP_80OPZXCUZO=-pBJSRTmHcQ2MnVTFov1meNbw18Q@mail.gmail.com> (raw)
In-Reply-To: <OS7PR01MB11964C077A3E61F4887DD3D09EAEFA@OS7PR01MB11964.jpnprd01.prod.outlook.com>
References: <OS7PR01MB119648846BA926C096CE016B6EAE3A@OS7PR01MB11964.jpnprd01.prod.outlook.com>
<CAHut+Pt4Tn1bQYCsYeUt_gtcSB-KOTtRB70SLghkpsjfKGsm7w@mail.gmail.com>
<OS7PR01MB119642CFE4F4DCF7FBBD040BBEAE0A@OS7PR01MB11964.jpnprd01.prod.outlook.com>
<CAHut+Pu-nxNice547eGEW2O3hdRcFbPYWF4HiqktZO19X3Ah-g@mail.gmail.com>
<OS7PR01MB11964628A74081DFCA442CBADEAE1A@OS7PR01MB11964.jpnprd01.prod.outlook.com>
<CAHut+PuuOKKW2oDT0Z8q+UXsiteS67j6G6075FC3aAxeR0cxHQ@mail.gmail.com>
<OSCPR01MB14966FC50CFB457938763AE09F5EEA@OSCPR01MB14966.jpnprd01.prod.outlook.com>
<OSCPR01MB14966B98E5119EBB261024ED2F5EEA@OSCPR01MB14966.jpnprd01.prod.outlook.com>
<[email protected]>
<OSCPR01MB1496614832F8014EC16FB78D2F5EEA@OSCPR01MB14966.jpnprd01.prod.outlook.com>
<[email protected]>
<OSCPR01MB14966EC12277712131EB8EDF1F5EEA@OSCPR01MB14966.jpnprd01.prod.outlook.com>
<OS7PR01MB11964C8FE9CDCC0F4C9110988EAEEA@OS7PR01MB11964.jpnprd01.prod.outlook.com>
<[email protected]>
<OS7PR01MB11964C077A3E61F4887DD3D09EAEFA@OS7PR01MB11964.jpnprd01.prod.outlook.com>
Hi Iwata-San,
Some v6 comments.
======
doc/src/sgml/bgworker.sgml
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>
======
src/backend/postmaster/bgworker.c
+
+
+/*
+ * 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.
======
src/include/postmaster/bgworker.h
+/*
+ * 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
view thread (67+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: [PROPOSAL] Termination of Background Workers for ALTER/DROP DATABASE
In-Reply-To: <CAHut+PtbOP_80OPZXCUZO=-pBJSRTmHcQ2MnVTFov1meNbw18Q@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox