RE: [PROPOSAL] Termination of Background Workers for ALTER/DROP DATABASE

From: "Aya Iwata (Fujitsu)" <iwata(dot)aya(at)fujitsu(dot)com>
To: 'Michael Paquier' <michael(at)paquier(dot)xyz>, "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>
Cc: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: RE: [PROPOSAL] Termination of Background Workers for ALTER/DROP DATABASE
Date: 2025-10-06 11:21:02
Message-ID: OS7PR01MB119648846BA926C096CE016B6EAE3A@OS7PR01MB11964.jpnprd01.prod.outlook.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

Thank you for your comments.

> On Mon, Oct 06, 2025 at 01:59:08AM +0000, Hayato Kuroda (Fujitsu) wrote:
> > Per my understanding, we already have a facility that terminates a
> background
> > worker, TerminateBackgroundWorker(). So, I'm afraid your proposal has
> already
> > been done by combining this function and ProcessUtility_hook.
>
> The main take that I am getting here from Iwata-san is that this would
> lead to less code duplication.

Yeah. My company sometimes wrote extensions which uses background workers,
and it was painful to implement the same part every time.

> lead to less code duplication. Another item, which you are not
> mentioning, is that this would be more flexible with bgworkers that
> have been starting dynamically, where shared_preload_libraries may not
> be used, still a bgworker would need to react. So the suggestion of a
> new API to control if a bgworker should be stopped like any other
> backend when there is a database activity worth it is a good one, as
> long as it is in line with what we do with normal backends.

Thanks for the agreement. Thus I want to proceed the patch.

> AcceptBackgroundWorkerCancel() is going backwards, IMO. Wouldn't it
> be better to pass it down as an option in bgw_flags, to mark that a
> bgworker connected to a database can be shutdown due to the effect of
> a database getting dropped or moved? There could be an argument
> behind using bgw_extra, but that would not be in line with the
> database connection argument which is a state defined when a bgworker
> is registered.

I updated my patch using bgw_flags to set whether accept to terminate bgworker or not.
And I also removed AcceptBackgroundWorkerCancel() function.
Please check my attached patch.

>
> > So, is the main benefit of the patch to shorten extensions codes which uses
> > bgworker?
>
> I'd ask for the addition of tests when it comes to such a facility,
> and your proposal has none of that. I would suggest worker_spi with
> an option that can be passed to worker_spi_launch().

I added the TAP test using worker_spi too.

Regards,
Aya Iwata
Fujitsu Limited

Attachment Content-Type Size
v0002-0001-Allow-background-workers-to-be-terminated.patch application/octet-stream 9.8 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Ranier Vilela 2025-10-06 11:33:10 Re: Fix misuse use of window_gettupleslot function (src/backend/executor/nodeWindowAgg.c)
Previous Message Jakub Wartak 2025-10-06 11:04:36 Re: Should we update the random_page_cost default value?