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 |
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? |