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

Hi,

On Wed, Oct 7, 2026 at 10:49 PM Masahiko Sawada <sawada.mshk@gmail.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.

> 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())

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