MIME-Version: 1.0
References: 
 <OS7PR01MB11964335F36BE41021B62EAE8EAE4A@OS7PR01MB11964.jpnprd01.prod.outlook.com>
 <OSCPR01MB149662AEA64F4E66F494C2584F5E3A@OSCPR01MB14966.jpnprd01.prod.outlook.com>
 <aOM02tVFtXmuR0M1@paquier.xyz>
 <OS7PR01MB119648846BA926C096CE016B6EAE3A@OS7PR01MB11964.jpnprd01.prod.outlook.com>
In-Reply-To: 
 <OS7PR01MB119648846BA926C096CE016B6EAE3A@OS7PR01MB11964.jpnprd01.prod.outlook.com>
From: Peter Smith <smithpb2250@gmail.com>
Date: Tue, 7 Oct 2025 14:12:06 +1100
Message-ID: 
 <CAHut+Pt4Tn1bQYCsYeUt_gtcSB-KOTtRB70SLghkpsjfKGsm7w@mail.gmail.com>
Subject: Re: [PROPOSAL] Termination of Background Workers for ALTER/DROP
 DATABASE
To: "Aya Iwata (Fujitsu)" <iwata.aya@fujitsu.com>
Cc: Michael Paquier <michael@paquier.xyz>,
	"Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>,
 pgsql-hackers <pgsql-hackers@postgresql.org>
Content-Type: text/plain; charset="UTF-8"
Archived-At: 
 <https://www.postgresql.org/message-id/CAHut%2BPt4Tn1bQYCsYeUt_gtcSB-KOTtRB70SLghkpsjfKGsm7w%40mail.gmail.com>
Precedence: bulk

Hi,

Here are some more minor review comments:

======
doc/src/sgml/bgworker.sgml

1. Typo?

s/damon/daemon/

======
src/backend/postmaster/bgworker.c

2.
+void
+CancelBackgroundWorkers(Oid databaseId, int cancel_flags)
+{
+ int slotno;
+ bool signal_postmaster = false;
+
+ LWLockAcquire(BackgroundWorkerLock, LW_EXCLUSIVE);
+
+ for (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) & cancel_flags)
+ {
+ PGPROC     *proc = BackendPidGetProc(slot->pid);
+
+ if (!proc)
+ continue;
+
+ /* 2nd, compare databaseId. */
+ if (proc->databaseId == databaseId)
+ {
+ /*
+ * Set terminate flag in shared memory, unless slot has
+ * been reused.
+ */
+ slot->terminate = true;
+ signal_postmaster = true;
+ }
+ }
+ }

2a.
Declare slotno as a 'for' loop variable.

~

2b.
There seem to be excessive conditions in the code. Is it better to
restructure with less, like:

for (int slotno = 0; ...)
{
  ...

  if (!slot->in_use)
    continue;

  if (slot flags are not set to drop)
    continue;
  proc = BackendPidGetProc(slot->pid);
  if (proc && proc->databaseId == databaseId)
  {
    slot->terminate = true;
    signal_postmaster = true;
  }
}

======
Kind Regards,
Peter Smith.
Fujitsu Australia