Re: Parallel autovacuum: DROP DATABASE WITH (FORCE) fails on the parallel workers - Mailing list pgsql-hackers

Hi,

On Tue, Oct 6, 2026 at 5:54 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
>
> > I've reviewed the patch and have one comment:

Thanks for reviewing the v2 patch.

> > +               PGPROC     *leader = proc->lockGroupLeader;
>
> > We read another backend's lockGroupLeader and dereference it without
> > holding the lock that protects the lock group fields, which seems
> > incorrect to me. If the worker exits after we read the pointer, the
> > leader's PGPROC could be recycled and we could check an unrelated
> > backend's role....
>
> On second thought, I think we don't need to acquire the leader's
> partition lock here.
>
> So I
> think it's enough to add a comment explaining why it's okay to read
> these fields without the lock. Sorry for the noise.

Please also note that pg_terminate_backend() doesn't take a lock
either when it reads PGPROC.roleId. I don't think that recycling
window is a big issue in practice.

> For the record, I've considered alternative solutions but the current
> approach is reasonable to me.
>
> A. Set parallel autovacuum worker's roleId to InvalidOid at its
> startup time. While it's good that it affects only parallel autovacuum
> cases, it would add another place to set the process's roleId. Also,
> there is still a small window where its roleId is
> BOOTSTRAP_SUPERUSERID. To remove the window completely, we need to
>  change parallel query code so that workers advertise the same roleId
> as their leader, which is too invasive.

I respectfully disagree with this option, for a couple of reasons.
It's invasive, of course, and resetting something to invalid outside
of the parallel workers infrastructure seems off to me. The recycling
window also seems to be accepted behavior (see my argument above).

> B. Ignore parallel workers in TerminateOtherDBBackends(). I think it's
> the second-best idea but I'm concerned that it would affect the
> existing behavior that terminates parallel workers.

I respectfully disagree with this option as well. Ignoring them is not
a good solution, IMHO.

> When it comes to the current idea, it changes
> TerminateOtherDBBackends() but the change would affect only the cases
> where the leader doesn't advertise its roleId, i.e., parallel
> autovacuum and parallel queries run by a bgworker connected without a
> user, and doesn't change the existing behavior of terminating parallel
> workers.

I took a closer look today, and here's what I have. More than the fix
itself, I would like to agree on the behavior first, since I think
this is a sensitive area. Please feel free to correct me.

Autovacuum workers (including slot sync workers and background
workers) that connect with no user name run as the bootstrap
superuser, but they leave PGPROC.roleId unset. Parallel workers
launched by them also run as the superuser, inherited from the leader,
but they do set PGPROC.roleId.

I looked at pg_terminate_backend() [1] first, because I think DROP
DATABASE FORCE has to agree with it per docs [2]. It lets only
superusers terminate superuser backends, except that
pg_signal_autovacuum_worker may terminate autovacuum workers. Whether
or not the predefined role includes the parallel workers launched by
autovacuum workers defines the scope of the fix.

Case 1 is a non-superuser with just pg_signal_backend. Case 2 is the
same role with pg_signal_autovacuum_worker.

pg_terminate_backend() on HEAD:
Case 1: autovacuum worker errors out, needs
pg_signal_autovacuum_worker. Parallel worker errors out, needs
SUPERUSER.
Case 2: autovacuum worker is terminated. Parallel worker errors out,
needs SUPERUSER.

DROP DATABASE FORCE on HEAD:
Case 1: autovacuum worker is terminated and the database drops.
Parallel worker errors out, needs SUPERUSER.
Case 2: same as case 1.

Do we want the forced database drop to match the behavior of
pg_terminate_backend()?

Do we want pg_signal_autovacuum_worker to cover the parallel workers
launched by autovacuum workers?

If yes to both, we need fixes in both places. I prefer this, unless I
am missing something.

If yes to the first and no to the second, we fix the forced database
drop and adjust the docs for pg_terminate_backend().

Thoughts?

[1] https://www.postgresql.org/docs/devel/functions-admin.html#FUNCTIONS-ADMIN-SIGNAL
[2] https://www.postgresql.org/docs/devel/sql-dropdatabase.html

--
Bharath Rupireddy
Amazon Web Services: https://aws.amazon.com