On Tue, Sep 29, 2026 at 7:46 PM Bharath Rupireddy <[email protected]> wrote: > > Hi, > > On Mon, Sep 28, 2026 at 11:39 AM Masahiko Sawada <[email protected]> > 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
