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.gmail.com
Best regards,
Hayato Kuroda
FUJITSU LIMITED
