Re: Parallel autovacuum: DROP DATABASE WITH (FORCE) fails on the parallel workers

On Wed, Oct 7, 2026 at 7:47 PM Bharath Rupireddy
<bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:
>
> Hi,
>
> On Tue, Oct 6, 2026 at 5:54 PM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)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.

These analyses match mine.

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

No. DROP DATABASE FORCE and pg_terminate_backend() purposefully work
differently for different purposes. See the comments in
TerminateOtherDBBackends():

/*
* Permissions checks relax the pg_terminate_backend checks in two
* ways, both by omitting the !OidIsValid(proc->roleId) check:
*
* - Accept terminating autovacuum workers, since DROP DATABASE
* without FORCE terminates them.
*
* - Accept terminating bgworkers. For bgworker authors, it's
* convenient to be able to recommend FORCE if a worker is blocking
* DROP DATABASE unexpectedly.
*
* Unlike pg_terminate_backend, we don't raise some warnings - like
* "PID %d is not a PostgreSQL server process", because for us already
* finished session is not a problem.
*/

If we had DROP DATABASE FORCE match the pg_terminate_backend(), DROP
DATABASE FORCE would also require pg_signal_autovacuum_worker to
terminate autovacuum workers. On the other hand, if we had
pg_terminate_backend() match DROP DATABASE FORCE, users with
pg_signal_backend would be able to terminate autovacuum workers,
making pg_signal_autovacuum_worker nonsense. Neither outcome makes
sense to me.

The consistency we need to maintain is between DROP DATABASE with
FORCE and without FORCE, and parallel autovacuum breaks it in v19,
which is what we need to fix for v19.

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

Not sure, but I think it's a separate topic for v20. Parallel workers
are not autovacuum worker processes. They are bgworkers internally and
shown as "parallel worker" in pg_stat_activity. So the current
behavior doesn't contradict the docs, and users can terminate the
autovacuum worker, which stops its parallel workers too.

Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com