RE: [PROPOSAL] Termination of Background Workers for ALTER/DROP DATABASE - Mailing list pgsql-hackers

From Hayato Kuroda (Fujitsu)
Subject RE: [PROPOSAL] Termination of Background Workers for ALTER/DROP DATABASE
Date
Msg-id OSCPR01MB14966250768B6E74CE5207864F5E0A@OSCPR01MB14966.jpnprd01.prod.outlook.com
Whole thread Raw
In response to RE: [PROPOSAL] Termination of Background Workers for ALTER/DROP DATABASE  ("Aya Iwata (Fujitsu)" <iwata.aya@fujitsu.com>)
List pgsql-hackers
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


pgsql-hackers by date:

Previous
From: shveta malik
Date:
Subject: Re: Improve pg_sync_replication_slots() to wait for primary to advance
Next
From: "Hayato Kuroda (Fujitsu)"
Date:
Subject: RE: Newly created replication slot may be invalidated by checkpoint