From 85acc84cf4a7a613d6ce8e2125a55bdc56291207 Mon Sep 17 00:00:00 2001 From: Zsolt Parragi Date: Thu, 15 Oct 2026 12:26:50 +0000 Subject: [PATCH v1] Fix restart of interruptible bgworkers after database commands TerminateBackgroundWorkersForDatabase() set the terminate flag of the workers' slots, which makes the postmaster unregister them regardless of bgw_restart_time. A restartable worker was gone until the next postmaster restart, even when the command failed afterwards, as with a DROP DATABASE blocked by another session, or left the database in place, as with CREATE DATABASE .. TEMPLATE. Add an interrupt flag to the worker slots, which makes the postmaster send SIGTERM to the worker without unregistering it, and use that instead, so that the worker is restarted as after any other exit with a non-zero exit code. Add a worker_spi test with a restartable worker. Oversight in f1e251be80a0. Author: Zsolt Parragi Backpatch-through: 19 --- doc/src/sgml/bgworker.sgml | 3 +++ src/backend/postmaster/bgworker.c | 25 +++++++++++++++++-- .../worker_spi/t/002_worker_terminate.pl | 19 ++++++++++++-- .../modules/worker_spi/worker_spi--1.0.sql | 3 ++- src/test/modules/worker_spi/worker_spi.c | 3 ++- 5 files changed, 47 insertions(+), 6 deletions(-) diff --git a/doc/src/sgml/bgworker.sgml b/doc/src/sgml/bgworker.sgml index 2affba74382..0f97c776d05 100644 --- a/doc/src/sgml/bgworker.sgml +++ b/doc/src/sgml/bgworker.sgml @@ -125,6 +125,9 @@ typedef struct BackgroundWorker TABLESPACE CREATE DATABASE + The worker is then restarted depending on + bgw_restart_time, as for any other exit + with a non-zero exit code. 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 3da2a90417f..5d969b4668f 100644 --- a/src/backend/postmaster/bgworker.c +++ b/src/backend/postmaster/bgworker.c @@ -65,7 +65,10 @@ dlist_head BackgroundWorkerList = DLIST_STATIC_INIT(BackgroundWorkerList); * As an exception, however, even when the slot is in use, regular backends * may set the 'terminate' flag for a slot, telling the postmaster not * to restart it. Once the background worker is no longer running, the slot - * will be released for reuse. + * will be released for reuse. Regular backends may also set the + * 'interrupt' flag, telling the postmaster to send SIGTERM to the worker + * without unregistering it, so that it is restarted as after any other + * exit with a non-zero exit code. * * In addition to coordinating with the postmaster, backends modifying this * data structure must coordinate with each other. Since they can take locks, @@ -80,6 +83,7 @@ typedef struct BackgroundWorkerSlot { bool in_use; bool terminate; + bool interrupt; pid_t pid; /* InvalidPid = not started yet; 0 = dead */ uint64 generation; /* incremented when slot is recycled */ BackgroundWorker worker; @@ -219,6 +223,7 @@ BackgroundWorkerShmemInit(void *arg) Assert(slotno < max_worker_processes); slot->in_use = true; slot->terminate = false; + slot->interrupt = false; slot->pid = InvalidPid; slot->generation = 0; rw->rw_shmem_slot = slotno; @@ -326,6 +331,18 @@ BackgroundWorkerStateChange(bool allow_new_workers) ReportBackgroundWorkerPID(rw); } } + + /* + * Someone can also set the interrupt flag, asking for the worker + * to be signaled once. A worker that is not running has nothing + * to be interrupted. + */ + if (slot->interrupt) + { + slot->interrupt = false; + if (rw->rw_pid != 0) + kill(rw->rw_pid, SIGTERM); + } continue; } @@ -1124,6 +1141,7 @@ RegisterDynamicBackgroundWorker(BackgroundWorker *worker, slot->pid = InvalidPid; /* indicates not started yet */ slot->generation++; slot->terminate = false; + slot->interrupt = false; generation = slot->generation; if (parallel) BackgroundWorkerData->parallel_register_count++; @@ -1420,6 +1438,9 @@ GetBackgroundWorkerTypeByPid(pid_t pid) /* * Terminate all background workers connected to the given database, if the * workers can be interrupted. + * + * The postmaster sends SIGTERM to the workers and restarts them depending + * on their bgw_restart_time. */ void TerminateBackgroundWorkersForDatabase(Oid databaseId) @@ -1449,7 +1470,7 @@ TerminateBackgroundWorkersForDatabase(Oid databaseId) proc = BackendPidGetProcWithLock(pid); if (proc && proc->databaseId == databaseId) { - slot->terminate = true; + slot->interrupt = true; signal_postmaster = true; elog(DEBUG1, "termination requested for worker (PID %d) on database %u", 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..6dea862fb05 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, $restartable) = @_; + $restartable //= 'false'; # 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, $restartable); )); # Check that the bgworker is initialized and napping. @@ -161,4 +162,18 @@ run_bgworker_interruptible_test( "DROP DATABASE renameddb", "DROP DATABASE", $pid); +# Test case 5: a restartable worker is restarted after its termination. +$pid = launch_bgworker($node, 'postgres', 5, 'true', 'true'); +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 pid <> $pid + AND wait_event = 'WorkerSpiMain';], + '1'); +is($result, 1, "restartable 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..9e6db6aea54 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, + restartable boolean DEFAULT false) 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..0c10b503f2d 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); + bool restartable = PG_GETARG_BOOL(5); memset(&worker, 0, sizeof(worker)); worker.bgw_flags = BGWORKER_SHMEM_ACCESS | @@ -415,7 +416,7 @@ 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 = restartable ? 1 : 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