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
