| From: | Nikhil Sontakke <nikhil(at)planetscale(dot)com> |
|---|---|
| To: | vignesh C <vignesh21(at)gmail(dot)com> |
| Cc: | Zhijie Hou <houzhijie22(at)gmail(dot)com>, shveta malik <shveta(dot)malik(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: Publication DDL can race with a concurrent UPDATE |
| Date: | 2026-10-14 10:31:49 |
| Message-ID: | CA+UBoq2=oUopxikh2BiMcWAEjEsci3BRyjaQ62Quxq+U9-PPkQ@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi Vignesh,
On Mon, Oct 12, 2026 at 3:07 PM vignesh C <vignesh21(at)gmail(dot)com> wrote:
> On Fri, 9 Oct 2026 at 15:16, Nikhil Sontakke <nikhil(at)planetscale(dot)com>
> wrote:
> >
> > Moving toward taking a stronger lock during DDL without penalizing DML
> sounds attractive.
> >
> > I tried v3 locally and found a lock-upgrade deadlock involving
> > pg_publication, even when the publications cover disjoint schemas.
>
> Thanks Nikhil for reporting the issue, I could reproduce the issue
> with the steps provided.
>
> > CreatePublication() already holds RowExclusiveLock on pg_publication when
> > LockTablesWithoutReplicaIdentity() requests ShareLock on the same
> catalog.
>
> The synchronization protocol in LockTablesWithoutReplicaIdentity() /
> RelationBuildPublicationDesc() was using the ordinary relation lock on
> pg_publication via LockRelationOid(). Any concurrent CREATE/ALTER/DROP
> PUBLICATION already holds a RowExclusiveLock on the same relation
> while modifying the catalog. Since RowExclusiveLock is
> self-compatible, these catalog locks do not block each other directly.
> However, when each session subsequently requests a ShareLock, that
> lock conflicts with the other session's still-held RowExclusiveLock,
> creating a deadlock cycle.
>
> > Although two ShareLock requests are compatible, each conflicts with the
> > other transaction's existing RowExclusiveLock.
> >
> > Here is a reproducer (the spec file is attached with this email as well):
> >
> > -- Setup
> > CREATE SCHEMA review_a;
> > CREATE TABLE review_a.t (id int PRIMARY KEY, val int);
> >
> > CREATE SCHEMA review_b;
> > CREATE TABLE review_b.t (id int PRIMARY KEY, val int);
> >
> > CREATE TABLE review_outside (id int, val int);
> > INSERT INTO review_outside VALUES (1, 1);
> >
> > -- Session 1
> > BEGIN;
> > UPDATE review_outside SET val = 2;
> >
> > Although review_outside is not published, its descriptor build acquires
> > RowExclusiveLock on pg_publication because it lacks replica identity.
> > That lock is retained until transaction end.
> >
> > -- Session 2
> > CREATE PUBLICATION review_pb FOR TABLES IN SCHEMA review_b;
> >
> > Session 2 blocks requesting ShareLock on pg_publication while also
> holding
> > the RowExclusiveLock acquired by CreatePublication().
> >
> > -- Session 1
> > CREATE PUBLICATION review_pa FOR TABLES IN SCHEMA review_a;
> >
> > This deadlocks: Session 1 requests ShareLock while Session 2 holds
> > RowExclusiveLock, and Session 2 is waiting for Session 1's
> RowExclusiveLock.
> >
> > The server reports both sessions waiting for ShareLock on relation
> > (pg_publication), each blocked by the other. The same sequence completes
> > without blocking on the unpatched build.
> >
> > This cycle involves only the publication catalog locks, so sorting the
> > user-table locks does not address it.
> >
> > A dedicated synchronization lock would avoid colliding with ordinary
> catalog
> > write locks and remove this additional lock-upgrade cycle.
>
> Agreed, How about we use dedicated LockDatabaseObject() tag instead
> of LockRelationOid():
> /* LockTablesWithoutReplicaIdentity(), publicationcmds.c */
> LockDatabaseObject(PublicationRelationId, InvalidOid, 0, ShareLock);
>
> /* RelationBuildPublicationDesc(), relcache.c */
> LockDatabaseObject(PublicationRelationId, InvalidOid, 0, RowExclusiveLock);
>
> Using InvalidOid as the object ID is safe as a sentinel because it is
> reserved and can never be a valid publication OID. Therefore, it
> cannot conflict with AlterPublication()'s existing per-publication
> lock on the same catalog.
>
> However, that would
> > not address the table/interlock deadlocks already acknowledged in the
> function
> > comment.
>
> Confirmed, and that is expected. The fix only removes the deadlock
> cycle between operations that do not actually share any objects.
> The attached top-up patch (on top of v3 patch shared at [1]) has the
> changes for the same.
> Thoughts?
>
>
Thanks for the fix. I tested v3 with your top-up patch, and my original
reproducer now completes successfully.
However, a variation still produces a lock-upgrade deadlock with disjoint
publication scopes. Both transactions
first UPDATE separate, unpublished tables without replica identity, then
create publications for separate
schemas containing only tables with primary keys:
-- Setup
CREATE SCHEMA test_a;
CREATE TABLE test_a.t (id int PRIMARY KEY, val int);
CREATE SCHEMA test_b;
CREATE TABLE test_b.t (id int PRIMARY KEY, val int);
CREATE TABLE test_outside_a (id int, val int);
CREATE TABLE test_outside_b (id int, val int);
INSERT INTO test_outside_a VALUES (1, 1);
INSERT INTO test_outside_b VALUES (1, 1);
-- Session 1
BEGIN;
UPDATE test_outside_a SET val = 2;
-- Session 2
BEGIN;
UPDATE test_outside_b SET val = 2;
-- Session 1: blocks
CREATE PUBLICATION test_pa FOR TABLES IN SCHEMA test_a;
-- Session 2: deadlocks
CREATE PUBLICATION test_pb FOR TABLES IN SCHEMA test_b;
The same sequence completes without blocking on the unpatched build.
Looks like this needs more thinking. Could we explore making the dedicated
interlock follow the publication’s
scope? Descriptor builds for tables without replica identity could take
writer-side locks on both a schema-specific
tag and a database-wide tag. Publication DDL for TABLES IN SCHEMA would
take a conflicting lock only on the
relevant schema tags, while ALL TABLES would use the database-wide tag.
Explicit table publications would continue
using the stronger table locks. This could avoid conflicts between
unrelated schema scopes, although overlapping
scopes could still deadlock.
Regards,
Nikhil
---
Nikhil Sontakke
PlanetScale Postgres Core Team
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Matthias Dötsch | 2026-10-14 10:36:49 | Perf: Specialize ExecIndexOnlyScan() like ExecSeqScan() |
| Previous Message | Shubhankar Anand Kulkarni | 2026-10-14 10:26:29 | Re: PG19: flat-copied RelOptInfos share extension_state with the original rel |