public inbox for [email protected]
help / color / mirror / Atom feedFrom: Aya Iwata (Fujitsu) <[email protected]>
To: 'Pavel Stehule' <[email protected]>
To: Michael Paquier <[email protected]>
Cc: Peter Smith <[email protected]>
Cc: Chao Li <[email protected]>
Cc: Hayato Kuroda (Fujitsu) <[email protected]>
Cc: pgsql-hackers <[email protected]>
Subject: RE: [PROPOSAL] Termination of Background Workers for ALTER/DROP DATABASE
Date: Thu, 25 Dec 2025 13:24:33 +0000
Message-ID: <OS7PR01MB11964C83E8E543743581250B0EAB3A@OS7PR01MB11964.jpnprd01.prod.outlook.com> (raw)
In-Reply-To: <CAFj8pRBk7zTKoxAAKAihNQuimztniWHPyRWHKp2iHq_2hH+dvQ@mail.gmail.com>
References: <[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>
<CAHut+PtbOP_80OPZXCUZO=-pBJSRTmHcQ2MnVTFov1meNbw18Q@mail.gmail.com>
<CAHut+Pt5BN0LDh7OzbNqh9+zqHBgsrLX+vh-gn+3FKYTFHMvhw@mail.gmail.com>
<TY3PR01MB11969CBD6DF3E0DB820AD4262EAE8A@TY3PR01MB11969.jpnprd01.prod.outlook.com>
<[email protected]>
<CAFj8pRB_FH-Pcth9XFcpY2OTasVOP-2DfYOsVxL38igw0O4hdg@mail.gmail.com>
<OS7PR01MB119647141272DBF9364A1C5AAEAADA@OS7PR01MB11964.jpnprd01.prod.outlook.com>
<CAFj8pRCRnN4SyZDPbQwTyKZ_kVHfwLG6udCXsXDTG_z9fWwYQg@mail.gmail.com>
<OS7PR01MB119648203BD748ED008FA5BF5EAABA@OS7PR01MB11964.jpnprd01.prod.outlook.com>
<CAFj8pRCn0=jn4yaeg1JoxxxnUNeXm1KCouES8Puq_GgBsXrNTQ@mail.gmail.com>
<OS7PR01MB1196405302B700736580DEE49EAA8A@OS7PR01MB11964.jpnprd01.prod.outlook.com>
<CAFj8pRBk7zTKoxAAKAihNQuimztniWHPyRWHKp2iHq_2hH+dvQ@mail.gmail.com>
Hi
https://www.postgresql.org/message-id/aPBoXXW3XuwiIsHG%40paquier.xyz
Based on previous discussions, we had configured DROP/ALTER DATABASE to
terminate only background workers with specific parameters set.
To reconsider the default behavior, I created the following patches:
0001 is the patch for the existing implementation.
0002 is the patch modified based on Pavel-san's suggestion.
The 0002 patch changes the default behavior to terminate background workers
when DROP/ALTER DATABASE is executed. It also includes a test set
for this change.
For background workers that should not be terminated, setting
the BGWORKER_PROTECTED parameter prevents this.
The relationship with the FORCE option during DROP DATABASE is as follows:
FORCE option present, flag present... TERMINATE
FORCE option present, flag absent... TERMINATE
FORCE option absent, flag present... Does not TERMINATE due to the flag
FORCE option absent, flag absent... TERMINATE
I would like to know your comments on which implementation is preferable.
Regards,
Aya Iwata
Attachments:
[application/octet-stream] v0011-0001-Allow-background-workers-to-be-terminated.patch (13.8K, 2-v0011-0001-Allow-background-workers-to-be-terminated.patch)
download | inline diff:
From f4d30412bd9588051a434a80404b077096a4cc98 Mon Sep 17 00:00:00 2001
From: "iwata.aya" <[email protected]>
Date: Thu, 11 Sep 2025 21:16:51 +0900
Subject: [PATCH v0011 1/2] Allow background workers to be terminated at DROP
DATABASE
---
doc/src/sgml/bgworker.sgml | 19 +++
src/backend/postmaster/bgworker.c | 40 +++++
src/backend/storage/ipc/procarray.c | 28 +++-
src/include/postmaster/bgworker.h | 8 +
src/test/modules/worker_spi/Makefile | 4 +
src/test/modules/worker_spi/meson.build | 4 +
.../worker_spi/t/002_worker_terminate.pl | 140 ++++++++++++++++++
.../modules/worker_spi/worker_spi--1.0.sql | 3 +-
src/test/modules/worker_spi/worker_spi.c | 5 +
9 files changed, 245 insertions(+), 6 deletions(-)
create mode 100644 src/test/modules/worker_spi/t/002_worker_terminate.pl
diff --git a/doc/src/sgml/bgworker.sgml b/doc/src/sgml/bgworker.sgml
index 2c393385a91..a7c238750fc 100644
--- a/doc/src/sgml/bgworker.sgml
+++ b/doc/src/sgml/bgworker.sgml
@@ -108,6 +108,25 @@ typedef struct BackgroundWorker
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><literal>BGWORKER_INTERRUPTABLE</literal></term>
+ <listitem>
+ <para>
+ <indexterm><primary>BGWORKER_INTERRUPTABLE</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>
+ </listitem>
+ </varlistentry>
+
</variablelist>
</para>
diff --git a/src/backend/postmaster/bgworker.c b/src/backend/postmaster/bgworker.c
index 8e1068969ae..8334b75548c 100644
--- a/src/backend/postmaster/bgworker.c
+++ b/src/backend/postmaster/bgworker.c
@@ -26,6 +26,7 @@
#include "storage/lwlock.h"
#include "storage/pmsignal.h"
#include "storage/proc.h"
+#include "storage/procarray.h"
#include "storage/procsignal.h"
#include "storage/shmem.h"
#include "tcop/tcopprot.h"
@@ -1399,3 +1400,42 @@ GetBackgroundWorkerTypeByPid(pid_t pid)
return result;
}
+
+/*
+ * Terminate all background workers connected to the given database, if they
+ * had requested it.
+ */
+void
+TerminateInterruptableBgWorkersByDbOid(Oid databaseId)
+{
+ bool signal_postmaster = false;
+
+ LWLockAcquire(BackgroundWorkerLock, LW_EXCLUSIVE);
+
+ /*
+ * Iterate through slots, looking for workers connected to the given
+ * database.
+ */
+ for (int slotno = 0; slotno < BackgroundWorkerData->total_slots; ++slotno)
+ {
+ BackgroundWorkerSlot *slot = &BackgroundWorkerData->slot[slotno];
+
+ if (slot->in_use &&
+ (slot->worker.bgw_flags & BGWORKER_INTERRUPTABLE))
+ {
+ PGPROC *proc = BackendPidGetProc(slot->pid);
+
+ if (proc && proc->databaseId == databaseId)
+ {
+ slot->terminate = true;
+ signal_postmaster = true;
+ }
+ }
+ }
+
+ LWLockRelease(BackgroundWorkerLock);
+
+ /* Make sure the postmaster notices the change to shared memory. */
+ if (signal_postmaster)
+ SendPostmasterSignal(PMSIGNAL_BACKGROUND_WORKER_CHANGE);
+}
diff --git a/src/backend/storage/ipc/procarray.c b/src/backend/storage/ipc/procarray.c
index f3a1603204e..de80422caea 100644
--- a/src/backend/storage/ipc/procarray.c
+++ b/src/backend/storage/ipc/procarray.c
@@ -56,11 +56,13 @@
#include "catalog/pg_authid.h"
#include "miscadmin.h"
#include "pgstat.h"
+#include "postmaster/bgworker.h"
#include "port/pg_lfind.h"
#include "storage/proc.h"
#include "storage/procarray.h"
#include "utils/acl.h"
#include "utils/builtins.h"
+#include "utils/injection_point.h"
#include "utils/lsyscache.h"
#include "utils/rel.h"
#include "utils/snapmgr.h"
@@ -3687,8 +3689,9 @@ CountUserBackends(Oid roleid)
* CountOtherDBBackends -- check for other backends running in the given DB
*
* If there are other backends in the DB, we will wait a maximum of 5 seconds
- * for them to exit. Autovacuum backends are encouraged to exit early by
- * sending them SIGTERM, but normal user backends are just waited for.
+ * for them to exit. Autovacuum backends and background workers are encouraged
+ * to exit early by sending them SIGTERM, but normal user backends are just
+ * waited for.
*
* The current backend is always ignored; it is caller's responsibility to
* check whether the current backend uses the given DB, if it's important.
@@ -3713,10 +3716,19 @@ CountOtherDBBackends(Oid databaseId, int *nbackends, int *nprepared)
#define MAXAUTOVACPIDS 10 /* max autovacs to SIGTERM per iteration */
int autovac_pids[MAXAUTOVACPIDS];
- int tries;
- /* 50 tries with 100ms sleep between tries makes 5 sec total wait */
- for (tries = 0; tries < 50; tries++)
+ /*
+ * Retry up to 50 times with 100ms between attempts (max 5s total). Can be
+ * reduced to 3 attempts (max 0.3s total) to speed up tests.
+ */
+ int ntries = 50;
+
+#ifdef USE_INJECTION_POINTS
+ if (IS_INJECTION_POINT_ATTACHED("reduce-ncounts"))
+ ntries = 3;
+#endif
+
+ for (int tries = 0; tries < ntries; tries++)
{
int nautovacs = 0;
bool found = false;
@@ -3766,6 +3778,12 @@ CountOtherDBBackends(Oid databaseId, int *nbackends, int *nprepared)
for (index = 0; index < nautovacs; index++)
(void) kill(autovac_pids[index], SIGTERM); /* ignore any error */
+ /*
+ * Terminate all background workers for this database, if they had
+ * requested it (BGWORKER_INTERRUPTABLE)
+ */
+ TerminateInterruptableBgWorkersByDbOid(databaseId);
+
/* sleep, then try again */
pg_usleep(100 * 1000L); /* 100ms */
}
diff --git a/src/include/postmaster/bgworker.h b/src/include/postmaster/bgworker.h
index 058667a47a0..e43881448d5 100644
--- a/src/include/postmaster/bgworker.h
+++ b/src/include/postmaster/bgworker.h
@@ -59,6 +59,13 @@
*/
#define BGWORKER_BACKEND_DATABASE_CONNECTION 0x0002
+/*
+ * Exit the bgworker if its database is involved in a CREATE, ALTER or DROP
+ * database command.
+ * Requires BGWORKER_SHMEM_ACCESS and BGWORKER_BACKEND_DATABASE_CONNECTION.
+ */
+#define BGWORKER_INTERRUPTABLE 0x0004
+
/*
* This class is used internally for parallel queries, to keep track of the
* number of active parallel workers and make sure we never launch more than
@@ -128,6 +135,7 @@ extern const char *GetBackgroundWorkerTypeByPid(pid_t pid);
/* Terminate a bgworker */
extern void TerminateBackgroundWorker(BackgroundWorkerHandle *handle);
+extern void TerminateInterruptableBgWorkersByDbOid(Oid databaseId);
/* This is valid in a running worker */
extern PGDLLIMPORT BackgroundWorker *MyBgworkerEntry;
diff --git a/src/test/modules/worker_spi/Makefile b/src/test/modules/worker_spi/Makefile
index 024b34cdbb3..e7c5c059e32 100644
--- a/src/test/modules/worker_spi/Makefile
+++ b/src/test/modules/worker_spi/Makefile
@@ -6,6 +6,10 @@ EXTENSION = worker_spi
DATA = worker_spi--1.0.sql
PGFILEDESC = "worker_spi - background worker example"
+EXTRA_INSTALL = src/test/modules/injection_points
+
+export enable_injection_points
+
TAP_TESTS = 1
ifdef USE_PGXS
diff --git a/src/test/modules/worker_spi/meson.build b/src/test/modules/worker_spi/meson.build
index d673ece48a0..5ba66051396 100644
--- a/src/test/modules/worker_spi/meson.build
+++ b/src/test/modules/worker_spi/meson.build
@@ -26,8 +26,12 @@ tests += {
'sd': meson.current_source_dir(),
'bd': meson.current_build_dir(),
'tap': {
+ 'env': {
+ 'enable_injection_points': get_option('injection_points') ? 'yes' : 'no',
+ },
'tests': [
't/001_worker_spi.pl',
+ 't/002_worker_terminate.pl'
],
},
}
diff --git a/src/test/modules/worker_spi/t/002_worker_terminate.pl b/src/test/modules/worker_spi/t/002_worker_terminate.pl
new file mode 100644
index 00000000000..5467e423fd9
--- /dev/null
+++ b/src/test/modules/worker_spi/t/002_worker_terminate.pl
@@ -0,0 +1,140 @@
+# Copyright (c) 2025, PostgreSQL Global Development Group
+
+# Test background workers can be terminated by db commands
+
+use strict;
+use warnings FATAL => 'all';
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More;
+
+# This test depends on injection points to detect whether background workers
+# remain.
+if ($ENV{enable_injection_points} ne 'yes')
+{
+ plan skip_all => 'Injection points not supported by this build';
+}
+
+# Ensure the worker_spi dynamic worker is launched on the specified database
+sub launch_bgworker
+{
+ my ($node, $database, $testcase, $request_terminate) = @_;
+ my $offset = -s $node->logfile;
+
+ # Launch a background worker on the given database
+ my $result = $node->safe_psql(
+ $database, qq(
+ SELECT worker_spi_launch($testcase, oid, 0, '{}', $request_terminate) IS NOT NULL
+ FROM pg_database WHERE datname = '$database';
+ ));
+ is($result, 't', "dynamic bgworker $testcase launched");
+
+ # Check the worker is surely initialized
+ $node->wait_for_log(
+ qr/LOG: worker_spi dynamic worker $testcase initialized with .*\..*/,
+ $offset);
+}
+
+# Run the given query and verify the background worker can be terminated
+sub run_db_command
+{
+ my ($node, $command, $testname) = @_;
+ my $offset = -s $node->logfile;
+
+ $node->safe_psql('postgres', $command);
+
+ $node->wait_for_log(
+ qr/terminating background worker \"worker_spi dynamic\" due to administrator command/,
+ $offset);
+
+ note("background worker can be terminated at $testname");
+}
+
+my $node = PostgreSQL::Test::Cluster->new('mynode');
+$node->init;
+$node->start;
+
+# Check if the extension injection_points is available, as it may be
+# possible that this script is run with installcheck, where the module
+# would not be installed by default.
+if (!$node->check_extension('injection_points'))
+{
+ plan skip_all => 'Extension injection_points not installed';
+}
+
+$node->safe_psql('postgres', 'CREATE EXTENSION worker_spi;');
+
+# Launch a background worker without BGWORKER_INTERRUPTABLE
+launch_bgworker($node, 'postgres', 0, "false");
+
+# Ensure CREATE DATABASE WITH TEMPLATE fails because background worker retains
+
+# The injection point 'reduce-ncounts' reduces the number of backend
+# retries, allowing for shorter test runs. See CountOtherDBBackends().
+$node->safe_psql('postgres', "CREATE EXTENSION injection_points;");
+$node->safe_psql('postgres',
+ "SELECT injection_points_attach('reduce-ncounts', 'error');");
+
+my $stderr;
+
+$node->psql(
+ 'postgres',
+ "CREATE DATABASE testdb WITH TEMPLATE postgres",
+ stderr => \$stderr);
+ok( $stderr =~
+ "source database \"postgres\" is being accessed by other users",
+ "background worker blocked the database creation");
+
+# Confirm a background worker is still running
+my $result = $node->safe_psql(
+ "postgres", qq(
+ SELECT count(1) FROM pg_stat_activity
+ WHERE backend_type = 'worker_spi dynamic';));
+
+is($result, '1',
+ "background worker is still running after CREATE DATABASE WITH TEMPLATE");
+
+# Terminate the worker for upcoming tests
+$node->safe_psql(
+ "postgres", qq(
+ SELECT pg_terminate_backend(pid)
+ FROM pg_stat_activity WHERE backend_type = 'worker_spi dynamic';));
+
+# The injection point won't be used anymore, release it.
+$node->safe_psql('postgres',
+ "SELECT injection_points_detach('reduce-ncounts');");
+
+# Ensure BGWORKER_INTERRUPTABLE allows background workers to be
+# terminated at some database manipulations.
+#
+# Testcase 1: CREATE DATABASE WITH TEMPLATE
+launch_bgworker($node, 'postgres', 1, "true");
+run_db_command(
+ $node,
+ "CREATE DATABASE testdb WITH TEMPLATE postgres",
+ "CREATE DATABASE WITH TEMPLATE");
+
+# Testcase 2: ALTER DATABASE RENAME
+launch_bgworker($node, 'testdb', 2, "true");
+run_db_command(
+ $node,
+ "ALTER DATABASE testdb RENAME TO renameddb",
+ "ALTER DATABASE RENAME");
+
+# Preparation for the next test; create another tablespace
+my $tablespace = PostgreSQL::Test::Utils::tempdir;
+$node->safe_psql('postgres',
+ "CREATE TABLESPACE test_tablespace LOCATION '$tablespace'");
+
+# Testcase 3: ALTER DATABASE SET TABLESPACE
+launch_bgworker($node, 'renameddb', 3, "true");
+run_db_command(
+ $node,
+ "ALTER DATABASE renameddb SET TABLESPACE test_tablespace",
+ "ALTER DATABASE SET TABLESPACE");
+
+# Testcase 4: DROP DATABASE
+launch_bgworker($node, 'renameddb', 4, "true");
+run_db_command($node, "DROP DATABASE renameddb", "DROP DATABASE");
+
+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 84deb6199f6..3d12de37bea 100644
--- a/src/test/modules/worker_spi/worker_spi--1.0.sql
+++ b/src/test/modules/worker_spi/worker_spi--1.0.sql
@@ -7,7 +7,8 @@
CREATE FUNCTION worker_spi_launch(index int4,
dboid oid DEFAULT 0,
roleoid oid DEFAULT 0,
- flags text[] DEFAULT '{}')
+ flags text[] DEFAULT '{}',
+ request_termination 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 1a21b8c8876..50d16aaf9a8 100644
--- a/src/test/modules/worker_spi/worker_spi.c
+++ b/src/test/modules/worker_spi/worker_spi.c
@@ -404,10 +404,15 @@ worker_spi_launch(PG_FUNCTION_ARGS)
Size ndim;
int nelems;
Datum *datum_flags;
+ bool request_termination = PG_GETARG_BOOL(4);
memset(&worker, 0, sizeof(worker));
worker.bgw_flags = BGWORKER_SHMEM_ACCESS |
BGWORKER_BACKEND_DATABASE_CONNECTION;
+
+ if (request_termination)
+ worker.bgw_flags |= BGWORKER_INTERRUPTABLE;
+
worker.bgw_start_time = BgWorkerStart_RecoveryFinished;
worker.bgw_restart_time = BGW_NEVER_RESTART;
sprintf(worker.bgw_library_name, "worker_spi");
--
2.39.3
[application/octet-stream] v0011-0002-Renamed-the-flag-to-BGWORKER_PROTECTED.patch (9.5K, 3-v0011-0002-Renamed-the-flag-to-BGWORKER_PROTECTED.patch)
download | inline diff:
From bc41ea1629f9e16f1c92c3475befecb0f6ef9a8c Mon Sep 17 00:00:00 2001
From: "iwata.aya" <[email protected]>
Date: Thu, 25 Dec 2025 17:16:55 +0900
Subject: [PATCH v0011 2/2] Renamed the flag to BGWORKER_PROTECTED and set
default behavior to Terminate.
---
doc/src/sgml/bgworker.sgml | 6 +-
src/backend/postmaster/bgworker.c | 2 +-
src/backend/storage/ipc/procarray.c | 2 +-
src/include/postmaster/bgworker.h | 2 +-
.../worker_spi/t/002_worker_terminate.pl | 117 ++++++++++++------
src/test/modules/worker_spi/worker_spi.c | 6 +-
6 files changed, 90 insertions(+), 45 deletions(-)
diff --git a/doc/src/sgml/bgworker.sgml b/doc/src/sgml/bgworker.sgml
index a7c238750fc..f56a1850ffb 100644
--- a/doc/src/sgml/bgworker.sgml
+++ b/doc/src/sgml/bgworker.sgml
@@ -109,11 +109,11 @@ typedef struct BackgroundWorker
</varlistentry>
<varlistentry>
- <term><literal>BGWORKER_INTERRUPTABLE</literal></term>
+ <term><literal>BGWORKER_PROTECTED</literal></term>
<listitem>
<para>
- <indexterm><primary>BGWORKER_INTERRUPTABLE</primary></indexterm>
- Requests termination of the background worker when its connected database is
+ <indexterm><primary>BGWORKER_PROTECTED</primary></indexterm>
+ Prevents 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:
diff --git a/src/backend/postmaster/bgworker.c b/src/backend/postmaster/bgworker.c
index 8334b75548c..4e0aa195140 100644
--- a/src/backend/postmaster/bgworker.c
+++ b/src/backend/postmaster/bgworker.c
@@ -1421,7 +1421,7 @@ TerminateInterruptableBgWorkersByDbOid(Oid databaseId)
BackgroundWorkerSlot *slot = &BackgroundWorkerData->slot[slotno];
if (slot->in_use &&
- (slot->worker.bgw_flags & BGWORKER_INTERRUPTABLE))
+ !(slot->worker.bgw_flags & BGWORKER_PROTECTED))
{
PGPROC *proc = BackendPidGetProc(slot->pid);
diff --git a/src/backend/storage/ipc/procarray.c b/src/backend/storage/ipc/procarray.c
index de80422caea..68b6aff151f 100644
--- a/src/backend/storage/ipc/procarray.c
+++ b/src/backend/storage/ipc/procarray.c
@@ -3780,7 +3780,7 @@ CountOtherDBBackends(Oid databaseId, int *nbackends, int *nprepared)
/*
* Terminate all background workers for this database, if they had
- * requested it (BGWORKER_INTERRUPTABLE)
+ * requested it (BGWORKER_PROTECTED)
*/
TerminateInterruptableBgWorkersByDbOid(databaseId);
diff --git a/src/include/postmaster/bgworker.h b/src/include/postmaster/bgworker.h
index e43881448d5..022b8130a64 100644
--- a/src/include/postmaster/bgworker.h
+++ b/src/include/postmaster/bgworker.h
@@ -64,7 +64,7 @@
* database command.
* Requires BGWORKER_SHMEM_ACCESS and BGWORKER_BACKEND_DATABASE_CONNECTION.
*/
-#define BGWORKER_INTERRUPTABLE 0x0004
+#define BGWORKER_PROTECTED 0x0004
/*
* This class is used internally for parallel queries, to keep track of the
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 5467e423fd9..ca59f3a836b 100644
--- a/src/test/modules/worker_spi/t/002_worker_terminate.pl
+++ b/src/test/modules/worker_spi/t/002_worker_terminate.pl
@@ -50,6 +50,21 @@ sub run_db_command
note("background worker can be terminated at $testname");
}
+# Confirm a background worker is still running
+sub confirm_bgworker_running
+{
+ my ($node, $dbname, $testinfo) = @_;
+
+ my $result = $node->safe_psql(
+ "$dbname", qq(
+ SELECT count(1) FROM pg_stat_activity
+ WHERE backend_type = 'worker_spi dynamic';));
+
+ is($result, '1',
+ "background worker is not stopped after $testinfo");
+
+}
+
my $node = PostgreSQL::Test::Cluster->new('mynode');
$node->init;
$node->start;
@@ -64,10 +79,15 @@ if (!$node->check_extension('injection_points'))
$node->safe_psql('postgres', 'CREATE EXTENSION worker_spi;');
-# Launch a background worker without BGWORKER_INTERRUPTABLE
+# Launch a background worker without BGWORKER_PROTECTED
launch_bgworker($node, 'postgres', 0, "false");
-# Ensure CREATE DATABASE WITH TEMPLATE fails because background worker retains
+# Ensure CREATE DATABASE WITH TEMPLATE sucseeds because background worker retains
+
+run_db_command(
+ $node,
+ "CREATE DATABASE testdb WITH TEMPLATE postgres",
+ "CREATE DATABASE WITH TEMPLATE");
# The injection point 'reduce-ncounts' reduces the number of backend
# retries, allowing for shorter test runs. See CountOtherDBBackends().
@@ -77,22 +97,20 @@ $node->safe_psql('postgres',
my $stderr;
+# Ensure BGWORKER_PROTECTED allows background workers not to be
+# terminated at some database manipulations.
+#
+# Testcase 1: CREATE DATABASE WITH TEMPLATE
+launch_bgworker($node, 'postgres', 1, "true");
$node->psql(
'postgres',
- "CREATE DATABASE testdb WITH TEMPLATE postgres",
+ "CREATE DATABASE testdb2 WITH TEMPLATE postgres",
stderr => \$stderr);
ok( $stderr =~
- "source database \"postgres\" is being accessed by other users",
+ "source database \"postgres\" is being accessed by other users",
"background worker blocked the database creation");
-# Confirm a background worker is still running
-my $result = $node->safe_psql(
- "postgres", qq(
- SELECT count(1) FROM pg_stat_activity
- WHERE backend_type = 'worker_spi dynamic';));
-
-is($result, '1',
- "background worker is still running after CREATE DATABASE WITH TEMPLATE");
+confirm_bgworker_running($node, "postgres", "CREATE DATABASE WITH TEMPLATE");
# Terminate the worker for upcoming tests
$node->safe_psql(
@@ -100,26 +118,23 @@ $node->safe_psql(
SELECT pg_terminate_backend(pid)
FROM pg_stat_activity WHERE backend_type = 'worker_spi dynamic';));
-# The injection point won't be used anymore, release it.
-$node->safe_psql('postgres',
- "SELECT injection_points_detach('reduce-ncounts');");
-
-# Ensure BGWORKER_INTERRUPTABLE allows background workers to be
-# terminated at some database manipulations.
-#
-# Testcase 1: CREATE DATABASE WITH TEMPLATE
-launch_bgworker($node, 'postgres', 1, "true");
-run_db_command(
- $node,
- "CREATE DATABASE testdb WITH TEMPLATE postgres",
- "CREATE DATABASE WITH TEMPLATE");
-
# Testcase 2: ALTER DATABASE RENAME
launch_bgworker($node, 'testdb', 2, "true");
-run_db_command(
- $node,
+$node->psql(
+ 'testdb',
"ALTER DATABASE testdb RENAME TO renameddb",
- "ALTER DATABASE RENAME");
+ stderr => \$stderr);
+ok( $stderr =~
+ "current database cannot be renamed",
+ "background worker blocked the alter database rename to");
+
+confirm_bgworker_running($node, "testdb", "ALTER DATABASE RENAME TO");
+
+# Terminate the worker for upcoming tests
+$node->safe_psql(
+ "testdb", qq(
+ SELECT pg_terminate_backend(pid)
+ FROM pg_stat_activity WHERE backend_type = 'worker_spi dynamic';));
# Preparation for the next test; create another tablespace
my $tablespace = PostgreSQL::Test::Utils::tempdir;
@@ -127,14 +142,44 @@ $node->safe_psql('postgres',
"CREATE TABLESPACE test_tablespace LOCATION '$tablespace'");
# Testcase 3: ALTER DATABASE SET TABLESPACE
-launch_bgworker($node, 'renameddb', 3, "true");
-run_db_command(
- $node,
- "ALTER DATABASE renameddb SET TABLESPACE test_tablespace",
- "ALTER DATABASE SET TABLESPACE");
+launch_bgworker($node, 'testdb', 3, "true");
+$node->psql(
+ 'testdb',
+ "ALTER DATABASE testdb SET TABLESPACE test_tablespace",
+ stderr => \$stderr);
+ok( $stderr =~
+ "cannot change the tablespace of the currently open database",
+ "background worker blocked the alter database set tablespace");
+
+confirm_bgworker_running($node, "testdb", "ALTER DATABASE SET TABLESPACE");
+
+# Terminate the worker for upcoming tests
+$node->safe_psql(
+ "testdb", qq(
+ SELECT pg_terminate_backend(pid)
+ FROM pg_stat_activity WHERE backend_type = 'worker_spi dynamic';));
# Testcase 4: DROP DATABASE
-launch_bgworker($node, 'renameddb', 4, "true");
-run_db_command($node, "DROP DATABASE renameddb", "DROP DATABASE");
+launch_bgworker($node, 'testdb', 4, "true");
+$node->psql(
+ 'testdb',
+ "DROP DATABASE testdb",
+ stderr => \$stderr);
+ok( $stderr =~
+ "cannot drop the currently open database",
+ "background worker blocked the database drop");
+
+confirm_bgworker_running($node, "testdb", "DROP DATABASE");
+
+# Terminate the worker
+$node->safe_psql(
+ "testdb", qq(
+ SELECT pg_terminate_backend(pid)
+ FROM pg_stat_activity WHERE backend_type = 'worker_spi dynamic';));
+
+
+# The injection point won't be used anymore, release it.
+$node->safe_psql('postgres',
+ "SELECT injection_points_detach('reduce-ncounts');");
done_testing();
diff --git a/src/test/modules/worker_spi/worker_spi.c b/src/test/modules/worker_spi/worker_spi.c
index 50d16aaf9a8..04ef060ff42 100644
--- a/src/test/modules/worker_spi/worker_spi.c
+++ b/src/test/modules/worker_spi/worker_spi.c
@@ -404,14 +404,14 @@ worker_spi_launch(PG_FUNCTION_ARGS)
Size ndim;
int nelems;
Datum *datum_flags;
- bool request_termination = PG_GETARG_BOOL(4);
+ bool prevent_termination = PG_GETARG_BOOL(4);
memset(&worker, 0, sizeof(worker));
worker.bgw_flags = BGWORKER_SHMEM_ACCESS |
BGWORKER_BACKEND_DATABASE_CONNECTION;
- if (request_termination)
- worker.bgw_flags |= BGWORKER_INTERRUPTABLE;
+ if (prevent_termination)
+ worker.bgw_flags |= BGWORKER_PROTECTED;
worker.bgw_start_time = BgWorkerStart_RecoveryFinished;
worker.bgw_restart_time = BGW_NEVER_RESTART;
--
2.39.3
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], [email protected]
Subject: RE: [PROPOSAL] Termination of Background Workers for ALTER/DROP DATABASE
In-Reply-To: <OS7PR01MB11964C83E8E543743581250B0EAB3A@OS7PR01MB11964.jpnprd01.prod.outlook.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