public inbox for [email protected]  

help / color / mirror / Atom feed

From: Hayato Kuroda (Fujitsu) <[email protected]>
To: Aya Iwata (Fujitsu) <[email protected]>
To: 'Peter Smith' <[email protected]>
Cc: Michael Paquier <[email protected]>
Cc: pgsql-hackers <[email protected]>
Subject: RE: [PROPOSAL] Termination of Background Workers for ALTER/DROP DATABASE
Date: Tue, 7 Oct 2025 11:49:13 +0000
Message-ID: <OSCPR01MB14966250768B6E74CE5207864F5E0A@OSCPR01MB14966.jpnprd01.prod.outlook.com> (raw)
In-Reply-To: <OS7PR01MB119642CFE4F4DCF7FBBD040BBEAE0A@OS7PR01MB11964.jpnprd01.prod.outlook.com>
References: <OS7PR01MB11964335F36BE41021B62EAE8EAE4A@OS7PR01MB11964.jpnprd01.prod.outlook.com>
	<OSCPR01MB149662AEA64F4E66F494C2584F5E3A@OSCPR01MB14966.jpnprd01.prod.outlook.com>
	<[email protected]>
	<OS7PR01MB119648846BA926C096CE016B6EAE3A@OS7PR01MB11964.jpnprd01.prod.outlook.com>
	<CAHut+Pt4Tn1bQYCsYeUt_gtcSB-KOTtRB70SLghkpsjfKGsm7w@mail.gmail.com>
	<OS7PR01MB119642CFE4F4DCF7FBBD040BBEAE0A@OS7PR01MB11964.jpnprd01.prod.outlook.com>

Dear Iwata-san,

Thanks for updating the patch. Comments:

```
+		/* Check worker slot. */
+		if (!slot->in_use)
+			continue;
```

The comment has less meaning. How about:
"Skip if the slot is not used"

```
+		/* 1st, check cancel flags. */
+		if (slot->worker.bgw_flags & BGWORKER_EXIT_AT_DATABASE_DROP)
```

Missing update 2b [1]. Also, since cancel flag does not exist anymore, the comment should be
Updated. How about something like:
"Skip if the background worker does not want to exit"

```
+			/* 2nd, compare databaseId. */
+			if (proc && proc->databaseId == databaseId)
```

Here should describes what are you trying to do. How about something like:
Checks the connecting database of the worker, and instruct the postmaster to terminate it if needed

```
+		/*
+		 * Cancel background workers by admin commands.
+		 */
+		CancelBackgroundWorkers(databaseId);
```

Since we removed the flag, the comment is outdated.

```
-
 typedef void (*bgworker_main_type) (Datum main_arg);
```

This change is not related with this patch.

```
@@ -361,7 +361,8 @@ _PG_init(void)
 	/* set up common data for all our workers */
 	memset(&worker, 0, sizeof(worker));
 	worker.bgw_flags = BGWORKER_SHMEM_ACCESS |
-		BGWORKER_BACKEND_DATABASE_CONNECTION;
+		BGWORKER_BACKEND_DATABASE_CONNECTION |
+		BGWORKER_EXIT_AT_DATABASE_DROP;
```

The new flag was added to both static and dynamic background workers. So, how about
testing both? I think it is enough to use one of case, like ALTER DATABASE SET TABLESPACE.

[1]: https://www.postgresql.org/message-id/CAHut%2BPt4Tn1bQYCsYeUt_gtcSB-KOTtRB70SLghkpsjfKGsm7w%40mail.g...

Best regards,
Hayato Kuroda
FUJITSU LIMITED

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]
  Subject: RE: [PROPOSAL] Termination of Background Workers for ALTER/DROP DATABASE
  In-Reply-To: <OSCPR01MB14966250768B6E74CE5207864F5E0A@OSCPR01MB14966.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