HI Iwata-San,
Here are some more review comments for v3.
======
doc/src/sgml/bgworker.sgml
1.
+ <para>
+ <indexterm><primary>BGWORKER_EXIT_AT_DATABASE_DROP</primary></indexterm>
+ Requests to terminate background worker when the database connected by
+ the background worker is changed. DBMS daemon can issue a termination
+ signal to the background worker.
+ This occurs only when significant changes affecting the entire database
+ take place.
+ Specifically, major changes include when the <command>DROP
DATABASE</command>,
+ <command>ALTER DATABASE RENAME TO</command>, and
+ <command>ALTER DATABASE SET TABLESPACE</command> commands are executed.
+ </para>
Here is a reworded version of that for your consideration
(AI-generated -- pls verify for correctness!):
<para>
<indexterm><primary>BGWORKER_EXIT_AT_DATABASE_DROP</primary></indexterm>
Requests termination of the background worker when the database it is
connected to undergoes significant changes. The postmaster will send a
termination signal to the background worker when any of the following
commands are executed: <command>DROP DATABASE</command>,
<command>ALTER DATABASE RENAME TO</command>, or
<command>ALTER DATABASE SET TABLESPACE</command>.
</para>
======
2.
+ for (int slotno = 0; slotno < BackgroundWorkerData->total_slots; ++slotno)
+ {
+ BackgroundWorkerSlot *slot = &BackgroundWorkerData->slot[slotno];
+
+ /* Check worker slot. */
+ if (!slot->in_use)
+ continue;
+
+ /* 1st, check cancel flags. */
+ if (slot->worker.bgw_flags & BGWORKER_EXIT_AT_DATABASE_DROP)
+ {
+ PGPROC *proc = BackendPidGetProc(slot->pid);
+
+ /* 2nd, compare databaseId. */
+ if (proc && proc->databaseId == databaseId)
+ {
+ /*
+ * Set terminate flag in shared memory, unless slot has
+ * been reused.
+ */
+ slot->terminate = true;
+ signal_postmaster = true;
+ }
+ }
+ }
IMO, most of those comments do not have any benefit because they only
repeat what is already obvious from the code.
2a.
+ /* Check worker slot. */
+ if (!slot->in_use)
Remove that one. It is the same as the code.
~
2b.
+ /* 1st, check cancel flags. */
+ if (slot->worker.bgw_flags & BGWORKER_EXIT_AT_DATABASE_DROP)
Remove that one. It is the same as the code
~
2c.
+ /* 2nd, compare databaseId. */
+ if (proc && proc->databaseId == databaseId)
Remove that one. It is the same as the code.
~
2d.
+ /*
+ * Set terminate flag in shared memory, unless slot has
+ * been reused.
+ */
This comment is a bit strange -- It seems slightly misplaced. IIUC,
the "unless slot has been reused" really is referring to the earlier
"slot->in_use". This whole comment may be better put immediately above
the 'for' loop as a short summary of the whole logic.
======
src/include/postmaster/bgworker.h
3.
+/*
+ * This flag means the bgworker must be exit when the connecting database is
+ * being dropped or moved.
+ * It requires both BGWORKER_SHMEM_ACCESS and
+ * BGWORKER_BACKEND_DATABASE_CONNECTION were passed too.
+ */
Not English. Needs rewording. Consider something like this:
/*
* Exit the bgworker when its database is dropped, renamed, or moved.
* Requires BGWORKER_SHMEM_ACCESS and BGWORKER_BACKEND_DATABASE_CONNECTION.
*/
======
Kind Regards,
Peter Smith.
Fujitsu Australia
