From b5c9c9cece66214ad4c7f6f016e93ec2adbdf08b Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Mon, 9 Nov 2026 12:31:49 +0900 Subject: [PATCH v3] Fix restart of interruptible bgworkers after database commands TerminateBackgroundWorkersForDatabase() set the terminate flag of the workers' slots, so the postmaster unregistered them regardless of bgw_restart_time, even if the database command failed afterwards. Let's signal the workers directly instead, like CountOtherDBBackends() does for autovacuum workers, so that they are restarted based on bgw_restart_time. This gives the option for extension developers to decide if an interruptible worker can be restarted or not, instead of always being thrown away. There can be an appeal in the restart if the bgworker relies on a database to connect to coming from a GUC, if its value can be reloaded. Oversight in f1e251be80a0. Author: Zsolt Parragi Author: Michael Paquier Reviewed-by: Aya Iwata Reviewed-by: Xuneng Zhou Discussion: https://postgr.es/m/CAN4CZFNP+_aVsTqxfyNLU+AtRZMJeDRAR9XRfjPTwsWs3TXVaQ@mail.gmail.com Backpatch-through: 19 --- doc/src/sgml/bgworker.sgml | 6 ++-- src/backend/postmaster/bgworker.c | 31 +++++++++++-------- .../worker_spi/t/002_worker_terminate.pl | 21 +++++++++++-- .../modules/worker_spi/worker_spi--1.0.sql | 3 +- src/test/modules/worker_spi/worker_spi.c | 4 ++- 5 files changed, 46 insertions(+), 19 deletions(-) diff --git a/doc/src/sgml/bgworker.sgml b/doc/src/sgml/bgworker.sgml index 2affba74382..f2627a807d3 100644 --- a/doc/src/sgml/bgworker.sgml +++ b/doc/src/sgml/bgworker.sgml @@ -115,8 +115,8 @@ typedef struct BackgroundWorker BGWORKER_INTERRUPTIBLE Requests termination of the background worker when its connected database is dropped, renamed, moved to a different tablespace, or used as a template for - CREATE DATABASE. Specifically, the postmaster sends a - termination signal when any of these commands affect the worker's database: + CREATE DATABASE. Specifically, a termination signal is + sent when any of these commands affect the worker's database: DROP DATABASE ALTER DATABASE RENAME @@ -125,6 +125,8 @@ typedef struct BackgroundWorker TABLESPACE CREATE DATABASE + The worker is then restarted or unregistered as after any other exit, + depending on its exit code and bgw_restart_time. Requires both BGWORKER_SHMEM_ACCESS and BGWORKER_BACKEND_DATABASE_CONNECTION. diff --git a/src/backend/postmaster/bgworker.c b/src/backend/postmaster/bgworker.c index 4aef037d5ae..e6948b99083 100644 --- a/src/backend/postmaster/bgworker.c +++ b/src/backend/postmaster/bgworker.c @@ -1419,17 +1419,20 @@ GetBackgroundWorkerTypeByPid(pid_t pid) /* * Terminate all background workers connected to the given database, if the - * workers can be interrupted. + * workers can be interrupted. The workers are signaled directly. */ void TerminateBackgroundWorkersForDatabase(Oid databaseId) { - bool signal_postmaster = false; + pid_t *pids; + int npids = 0; elog(DEBUG1, "attempting worker termination for database %u", databaseId); - LWLockAcquire(BackgroundWorkerLock, LW_EXCLUSIVE); + pids = palloc_array(pid_t, BackgroundWorkerData->total_slots); + + LWLockAcquire(BackgroundWorkerLock, LW_SHARED); /* * Iterate through slots, looking for workers connected to the given @@ -1448,20 +1451,22 @@ TerminateBackgroundWorkersForDatabase(Oid databaseId) LWLockAcquire(ProcArrayLock, LW_SHARED); proc = BackendPidGetProcWithLock(pid); if (proc && proc->databaseId == databaseId) - { - slot->terminate = true; - signal_postmaster = true; - - elog(DEBUG1, "termination requested for worker (PID %d) on database %u", - (int) pid, databaseId); - } + pids[npids++] = pid; LWLockRelease(ProcArrayLock); } } LWLockRelease(BackgroundWorkerLock); - /* Make sure the postmaster notices the change to shared memory. */ - if (signal_postmaster) - SendPostmasterSignal(PMSIGNAL_BACKGROUND_WORKER_CHANGE); + /* + * Signal the workers, avoiding kill() while holding the locks. + */ + for (int i = 0; i < npids; i++) + { + elog(DEBUG1, "termination requested for worker (PID %d) on database %u", + (int) pids[i], databaseId); + (void) kill(pids[i], SIGTERM); /* ignore any error */ + } + + pfree(pids); } 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 69972a838dc..67f7832fd52 100644 --- a/src/test/modules/worker_spi/t/002_worker_terminate.pl +++ b/src/test/modules/worker_spi/t/002_worker_terminate.pl @@ -19,12 +19,13 @@ if ($ENV{enable_injection_points} ne 'yes') # Returns the PID of the worker launched. sub launch_bgworker { - my ($node, $database, $testcase, $interruptible) = @_; + my ($node, $database, $testcase, $interruptible, $restart_time) = @_; + $restart_time //= -1; # Launch a background worker on the given database. my $pid = $node->safe_psql( 'postgres', qq( - SELECT worker_spi_launch($testcase, '$database'::regdatabase, 0, '{}', $interruptible); + SELECT worker_spi_launch($testcase, '$database'::regdatabase, 0, '{}', $interruptible, $restart_time); )); # Check that the bgworker is initialized and napping. @@ -161,4 +162,20 @@ run_bgworker_interruptible_test( "DROP DATABASE renameddb", "DROP DATABASE", $pid); +# Test case 5: restart after termination. Restart time at 0 implies zero +# delay in the restart. The old worker is known to have exited, so do not +# rely on the PID of the new worker being different, as it could be reused. +$pid = launch_bgworker($node, 'postgres', 5, 'true', 0); +run_bgworker_interruptible_test( + $node, + "CREATE DATABASE testdb2 WITH TEMPLATE postgres", + "CREATE DATABASE WITH TEMPLATE, restartable worker", $pid); +$result = $node->poll_query_until( + 'postgres', + qq[SELECT count(*) FROM pg_stat_activity + WHERE backend_type = 'worker_spi dynamic' + AND wait_event = 'WorkerSpiMain';], + '1'); +is($result, 1, "bgworker restarted after termination"); + done_testing(); diff --git a/src/test/modules/worker_spi/worker_spi--1.0.sql b/src/test/modules/worker_spi/worker_spi--1.0.sql index f5e9621b0d1..8d112a94327 100644 --- a/src/test/modules/worker_spi/worker_spi--1.0.sql +++ b/src/test/modules/worker_spi/worker_spi--1.0.sql @@ -8,7 +8,8 @@ CREATE FUNCTION worker_spi_launch(index int4, dboid oid DEFAULT 0, roleoid oid DEFAULT 0, flags text[] DEFAULT '{}', - interruptible boolean DEFAULT false) + interruptible boolean DEFAULT false, + restart_time int4 DEFAULT -1) RETURNS pg_catalog.int4 STRICT AS 'MODULE_PATHNAME' LANGUAGE C; diff --git a/src/test/modules/worker_spi/worker_spi.c b/src/test/modules/worker_spi/worker_spi.c index b635a486349..668c582de43 100644 --- a/src/test/modules/worker_spi/worker_spi.c +++ b/src/test/modules/worker_spi/worker_spi.c @@ -406,6 +406,7 @@ worker_spi_launch(PG_FUNCTION_ARGS) int nelems; Datum *datum_flags; bool interruptible = PG_GETARG_BOOL(4); + int restart_time = PG_GETARG_INT32(5); memset(&worker, 0, sizeof(worker)); worker.bgw_flags = BGWORKER_SHMEM_ACCESS | @@ -415,7 +416,8 @@ worker_spi_launch(PG_FUNCTION_ARGS) worker.bgw_flags |= BGWORKER_INTERRUPTIBLE; worker.bgw_start_time = BgWorkerStart_RecoveryFinished; - worker.bgw_restart_time = BGW_NEVER_RESTART; + worker.bgw_restart_time = restart_time >= 0 ? + restart_time : BGW_NEVER_RESTART; sprintf(worker.bgw_library_name, "worker_spi"); sprintf(worker.bgw_function_name, "worker_spi_main"); snprintf(worker.bgw_name, BGW_MAXLEN, "worker_spi dynamic worker %d", i); -- 2.56.0