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

On Mon, Oct 13, 2025 at 9:28 AM Peter Smith <smithpb2250(at)gmail(dot)com> wrote:
>
> 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?
>

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.