Hi Chao-san, Michael san Thank you for your comments! To accept your comment, I updated patch to v0008.
> From: Chao Li <[email protected]> > Sent: Wednesday, October 15, 2025 12:37 PM ... > By searching for “ByOid”, we can get some existing examples: > > ObjectAddress > RefreshMatViewByOid(Oid matviewOid, bool is_create, bool skipData, > bool concurrent, const char *queryString, > QueryCompletion *qc) > > The function name clearly tells refresh MatView by Oid, so the oid in > parameter is an old of mat view. > > ResultRelInfo * > ExecLookupResultRelByOid(ModifyTableState *node, Oid resultoid, > bool missing_ok, bool update_cache) > > The function name indicates ResultRel, so the oid is a result oid. > > AccessMethodInfo * > findAccessMethodByOid(Oid oid) > > The function name tells to find access method, the the oid is an access > method’s OID. > > You can find more … > > But in this patch, the function name only indeeds “terminate background > workers”, while the oid is a database oid. Maybe we can rename the > function to “TerminateDatabaseBgWorkersByOid()”. Thank you. I changed the function name to "'TerminateBgWorkersByDbOid". I prefer this name because there are not official terminology "Database background worker" and it's shorter. > -----Original Message----- > From: Michael Paquier <[email protected]> > Sent: Thursday, October 16, 2025 12:55 PM ... > On Wed, Oct 15, 2025 at 02:48:43AM +0000, Aya Iwata (Fujitsu) wrote: > + * Exit the bgworker when its database is dropped, renamed, moved to a > + * different tablespace, or used as a template for CREATE DATABASE. > > I don't think that we need to list all these operations in details > here. We could just say "if its database is involved in a CREATE, > ALTER or DROP database command". The docs should provide these > details, of course. Thank you. I fixed this .h file comment. > > +#define BGWORKER_EXIT_AT_DATABASE_CHANGE 0x0004 > > Flag name works here. Sorry, I cannot follow. Please tell me more details about this comment. > # XXX This spends more than 5 seconds because the backend retries counting > # number of connecting processes 50 times. See CountOtherDBBackends(). > > And that's annoying. Let's activate what I call the cheat mode for > this one: an injection point that, if defined, enforces a lower number > of tries when we loop over the workers to stop. That would make the > test much faster when using a worker that should not be stopped, > without impacting the coverage. I tried to implement your idea. Thanks Kuroda-san to help it. > I suspect that your new test 002_worker_terminate.pl has a race > condition in run_db_command(): are you sure that the bgworker has > enough time to be reported as stopped in the server logs once > safe_psql() finishes to run the database command given by the caller? > On very slow and/or loaded machines, particularly, that could hurt the > stability. It seems to me that this should use a wait_for_log() > instead of a log_contains(), waiting for the worker to be reported as > stopped depending on the command executed. I fixed this test to use wait_for_log() instead of log_contains(). > Shouldn't this test also check that worker 0 (the one that does not > have the flag set) is still running at the end of the test? I assume > that querying pg_stat_activity would be enough at the end of the > script. Added. I cannot find a good way to clarify the worker is "worker 0" from the pg_stat_activity, backend_type does not have the information. Thus I used the string "worker_spi dynamic" as the key. Best regards, Aya Iwata Fujitsu Limited
v0008-0001-Allow-background-workers-to-be-terminated.patch
Description: v0008-0001-Allow-background-workers-to-be-terminated.patch
