Re: Publication DDL can race with a concurrent UPDATE

On Sun, Oct 4, 2026 at 5:56 PM Zhijie Hou <houzhijie22(at)gmail(dot)com> wrote:
>
> Hi,
>
> On Tue, Sep 29, 2026 at 6:12 PM vignesh C <vignesh21(at)gmail(dot)com> wrote:
> >
> > On Tue, 8 Sept 2026 at 10:54, Zhijie Hou (Fujitsu)
> > <houzj(dot)fnst(at)fujitsu(dot)com> wrote:
> > > I think this fix is not sufficient, as it does not address the ALTER PUBLICATION
> > > SET (options) cases, where the publication action can also be altered
> > > concurrently with DMLs, IIUC. The publication data in the relcache is also
> > > affected by pubaction changes, so those should be blocked as well.
> > >
> > > Addressing the above should be sufficient for the row filter and column list
> > > cases. However, for the replica identity check on UPDATE and DELETE operations,
> > > further analysis may be needed - especially for the TABLES IN SCHEMA and ALL
> > > TABLES cases, where tables are not explicitly published.
> >
> > One approach could be:
> > For "TABLES IN SCHEMA", lock the schema's namespace OID.
> > LockSchemaList() already takes a lock on the schema to prevent DROP
> > SCHEMA, so upgrade it from AccessShareLock to ShareRowExclusiveLock.
> > This keeps the existing protection and also prevents concurrent
> > writers from racing with CREATE PUBLICATION ... FOR TABLES IN SCHEMA
> > and ALTER PUBLICATION ... ADD/SET TABLES IN SCHEMA. For "FOR ALL
> > TABLES", lock the pg_publication relation with ShareRowExclusiveLock.
> > Take this lock in CreatePublication() when enabling FOR ALL TABLES,
> > and in AlterPublicationAllFlags() when changing puballtables from
> > false to true. On the DML side(UPDATE and DELETE),
> > CheckCmdReplicaIdentity() takes a matching RowExclusiveLock on the
> > table's namespace and pg_publication before using the publication
> > descriptor. This is needed only for tables without a local replica
> > identity; tables with a replica identity or REPLICA IDENTITY FULL are
> > not affected by this race.
> > RowExclusiveLock is self-compatible, so normal concurrent DML does not
> > block other DML. It conflicts with the ShareRowExclusiveLock taken by
> > the publication DDL, ensuring that the DDL and DML cannot race.
> > The attached POC demonstrates the changes for the same.
> >
> > Does this approach look reasonable, or is there a better way to handle
> > this synchronization?
>
> After considering more...
>
> I think adding a heavy lock acquisition per DML statement sounds risky,
> especially since this is a hot path that every table lacking a replica identity
> will go through, whether or not it's published. Acquiring a lock even when no
> conflict exists is not free either and the lock is held until transaction end.
> I recall an argument [1] in another thread that adding an additional
> heavyweight lock per transaction could limit the maximum achievable TPS. And
> considering it's not rare to run pgbench with many backends doing UPDATE/DELETE
> on tables lacking a replica identity, this needs more thoughts.

Rather than assuming there will be a regression with v2, I think it
would be good to test it first.

We could run pgbench with UPDATE/DELETE workloads on two types of
tables: one with a primary key (RI), and another with no RI. The table
with a RI should ideally see no impact. While the second case could
see some TPS regression, quantifying it would be useful. This data
will help us decide the next steps and approach to choose.

> I'm trying to explore whether we can narrow the scope to the DDL side as much
> as possible, to avoid adding such a risky change to the hot path.
>
> Here is one idea, it's not perfect but I feel it's worth discussing.
>
> The idea is to make TABLES IN SCHEMA or ALL TABLES publications lock the tables
> only when they have neither a replica identity index nor replica identity FULL,
> to prevent concurrent DML on these tables to execute. We only lock such tables,
> because if we allow DML to write logs on these tables, the changes cannot be
> replayed on the subscriber side due to lack of identity key. Nothing is locked
> when the publication does not publish UPDATE or DELETE, since only those
> actions require a replica identity. For tables named explicitly in a
> publication, we increase the lock level as Vignesh's patch does, and
> additionally lock their partition trees, the explicitly listed members when SET
> (publish = ...) enables UPDATE or DELETE, and the tree of a dropped EXCEPT
> exclusion.
>
> For new tables created concurrently, we can take a conflicting lock on the
> publication catalog only when building the pubcache in
> RelationBuildPublicationDesc(). The descriptor is built exactly when the
> relcache entry is fresh or was invalidated, which means it's possible the table
> was just created. The DDL holds the catalog lock until commit, so it cannot
> commit between the descriptor build and the WAL write. This is the only lock
> taken on the DML side, and steady-state DML never touches it. This also
> protects the cases of tables that had a replica identity: if the identity index
> is removed from the table, the cache will be rebuilt and the writer will take
> the catalog lock, serializing with a concurrent publication DDL that may not
> have seen the removal.
>
> In the worst case this could still lock a large number of tables, so the DDL
> counts them and raises a clear error ("too many tables without a replica
> identity in the publication") instead of a later "out of shared memory". This
> is expected to be very rare, and since UPDATEs/DELETEs on those tables fail
> anyway, the error should be acceptable because it hints that the publication
> contains many tables that can't actually be published (since they lack RI).
>
> I'm sharing two POC patches for reference and discussion (they're not in great
> shape and might have some bugs, but they basically implement the idea above).
> 0001 simply increases the lock level for explicitly included tables (FOR TABLE
> a, b) in a publication, and additionally locks the child table when only the
> parent is published. 0002 handles the TABLES IN SCHEMA and ALL TABLES cases,
> locking tables lacking RI as described above.
>
> [1] https://www.postgresql.org/message-id/CA%2BTgmoZzoLzcbrP9%3DY3E%3D6xfyp%3DuH8CobowhuDV8fLbisQtbyA%40mail.gmail.com
>
> Best Regards,
> Zhijie Hou