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

On Tue, Sep 29, 2026 at 7:46 PM Bharath Rupireddy
<bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:
>
> Hi,
>
> On Mon, Sep 28, 2026 at 11:39 AM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
> >
> > Thank you for the report and the patch!
>
> Thanks for taking a look at it.
>
> > IIUC the issue stems from the fact that the leader and its workers
> > advertise different roleIds (InvalidOid and BOOTSTRAP_SUPERUSERID). I
> > think the same issue can be reproduced in other cases. For instance,
> > suppose that a bgworker connecting to the database via
> > BackgroundWorkerInitializeConnection(dbname, NULL, 0) runs a parallel
> > query, the leader's roleId is InvalidOid whereas the parallel query
> > workers have BOOTSTRAP_SUPERUSERID. I think we should fix it as well
> > and backpatch the fix to 14.
>
> Good catch. Yes, it happens there too. I verified it locally with a
> simple test module that launches a background worker and starts a
> parallel worker to run a query. I agree the fix needs to be
> back-patched through 14.
>
> > I have some review comments on the proposed patch:
> >
> > + if (leader != NULL && leader != proc &&
> > + leader->backendType == B_AUTOVAC_WORKER)
> > + roleId = InvalidOid;
> >
> > I think we should check a lock group member with its leader's roleId
> > instead of unconditionally using InvalidOid. That would
> > straightforwardly fix the inconsistency between the leader and the
> > workers.
> >
> > if (leader != NULL && leader != proc &&
> > leader->databaseId == databaseId)
> > roleId = leader->roleId;
> >
> > To fix this issue not only in autovacuum cases, the backendType check
> > should be removed.
>
> Agreed, that is better. Checking a parallel worker against its
> leader's roleId is the right choice here, since both are members of
> the same lock group. It gives the expected behavior, terminating both
> the leader and the workers for DROP DATABASE FORCE, which both run as
> the bootstrap superuser.
>
> Please find the attached v2 patch. I plan to share patches for the
> back branches without the TAP test for branches < PG19 unless there
> are any comments.

I've reviewed the patch and have one comment:

+ PGPROC *leader = proc->lockGroupLeader;
+ Oid roleId = proc->roleId;
+
...
+ */
+ if (leader != NULL && leader != proc &&
+ leader->databaseId == databaseId)
+ roleId = leader->roleId;

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. I think that we can acquire the leader's partition
lock and recheck if the leader is still the leader we're looking for,
like BecomeLockGroupMember() does. It would require one more LWLock
acquisition per lock group member but probably we can live with it as
DROP DATABASE FORCE isn't a hot path. What do you think?

Regards,

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