Re: Publication DDL can race with a concurrent UPDATE

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.

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