On Fri, Dec 26, 2025 at 10:17:27AM +0000, Ryo Matsumura (Fujitsu) wrote: > +1 to Allow-background-workers-to-be-terminated > > The result is same, so I think it's better to prioritize compatibility. > > PGWORKER_PROTECTED would be used in scenarios like the following: > Existing features are probably not designed to be forcibly stopped. > Therefore, all existing features should have PROTECTED applied to them. > Most newly implemented features will also have PROTECTED applied because it > requires less thought and is safer. > Only considerate developers of features that can easily guarantee safety > would adopt the default.
We could design things so as we have a second flag to force a bgworker to be non-interuptible, then we could force that either the interruptible flag or the non-interruptible flag should be set. What is mentioned as a problem is that 0 implies that the non-interruptible is enforced. I don't think that we would have much to gain by doing that, as it would just lead to extension breakages that we can avoid. > In conclusion, this is no different from BGWORKER_INTERRUPTABLE. > Therefore, I think it's better to prioritize compatibility. Looking finally at the patch, I like the simplicity of what you are doing here. + Requires both <literal>BGWORKER_SHMEM_ACCESS</literal> and + <literal>BGWORKER_BACKEND_DATABASE_CONNECTION</literal>. SanityCheckBackgroundWorker() does not enforce this check when registering a bgworker. But shouldn't we do so when the interruptible flag is set? +void +TerminateInterruptableBgWorkersByDbOid(Oid databaseId) Fine by me to aim for simplicity with this interface, discarding my previous fancy comment about the extensibility we could do here. Matsumura-san and I have also discussed a bit that offline at the last JPUG, where I said that I'm OK with simpler at the end. One issue with the test as written, as of run_db_command(), is that we make sure that a worker is stopped by scanning the output of the logs. This approach may detect incorrect patterns, unfortunately. For example, if the termination logic has a bug it may be possible that the worker found as terminated is the first one created by the test, which we expect to always run. While the log is mandatory to have, I have a suggestion to make that even better: let's keep track in run_db_command() of the PIDs of the worker processes we expect to exist after running each database command, then make sure that the list of PIDs match with what we expect. This is a bit simpler in the case of this test as we only expect one matching PID. -- Michael
signature.asc
Description: PGP signature
