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

On Thu, Oct 8, 2026 at 10:27 AM Bharath Rupireddy
<bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:
>
> Hi,
>
> On Wed, Oct 7, 2026 at 10:49 PM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
> >
> > > 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():
>
> My bad. I missed that. Thanks for pointing to that.
>
> > /*
> > * 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.
>
> I read the commit 372700cf3067, re-read the docs, and found the hidden
> note. We took exception from pg_terminate_backend()'s behavior for
> autovacuum workers (of course for bg workers as well) allowing the
> non-superuser role to terminate the autovacuum workers and force drop
> the database. So, it now needs to also include the parallel workers
> started by autovacuum workers. I will add a comment about this in the
> hidden docs and the comments in TerminateOtherDBBackends().
>
> > 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.
>
> Yes, that's correct with the v2 patch. If we were to fix it for
> parallel workers started by any bg worker (which I think we should),
> the fix also needs to be back-patched. If agreed, I will check how far
> back it needs to be back-patched and prepare patches.

I'd fix it only in v19 for now. While the problem exists in the back
branches too, it has never been reported in the field. Also the next
minor release will be the final one for v14, there would be no chance
to fix it there if this change turned out to be wrong. I think that If
we get a report in the future, we can backpatch it.

>
> > Also, I think it's better to add the !OidIsValid(leader->roleId) check
> > to the if statement (and set roleId = InvalidOid directly). That way,
> > the change would affect only the specific case where the leader
> > doesn't advertise its roleId while its workers do, which seems better
> > to me for backpatching than creating a generic rule "if the leader and
> > the worker have different roles, the worker inherits the leader's
> > roleId only when being terminated".
>
> Does the following work? I still think setting roleId with
> leader->roleId is good for readability purposes (it avoids a line in
> the comment).
>
> @@ -3900,14 +3900,30 @@ TerminateOtherDBBackends(Oid databaseId)
>
> if (proc != NULL)
> {
> - if (superuser_arg(proc->roleId) && !superuser())
> + PGPROC *leader = proc->lockGroupLeader;
> + Oid roleId = proc->roleId;
> +
> + /*
> + * An autovacuum worker and a
> background worker with no user
> + * name advertise no role and they run
> as bootstrap superuser (see
> + * InitializeSessionUserIdStandalone()
> callsites in
> + * postinit.c). While the parallel
> workers they launch inherit
> + * the same superuser and advertise it
> in their PGPROC. Accept
> + * such parallel workers as well.
> + */
> + if (leader != NULL && leader != proc &&
> + !OidIsValid(leader->roleId) &&
> + leader->databaseId == databaseId)
> + roleId = leader->roleId;
> +
> + if (superuser_arg(roleId) && !superuser())
>

Fine with me. But probably can we remove leader->databaseId ==
databaseId check? It seems redundant for parallel workers.

Regards,

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