Re: Publication DDL can race with a concurrent UPDATE

On Tue, 6 Oct 2026 at 17:50, Zhijie Hou <houzhijie22(at)gmail(dot)com> wrote:
>
> Hi,
>
> On Tue, Oct 6, 2026 at 5:23 PM shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:
> >
> > On Sun, Oct 4, 2026 at 5:56 PM Zhijie Hou <houzhijie22(at)gmail(dot)com> wrote:
> > >
> > > 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 think the main point is that adding a new operation to a hot path like DML
> requires a strong reason to convince ourselves and others. Even if the effect
> is not visible in the test data, it's clear that acquiring a heavyweight lock
> will add quite a few CPU cycles. Moreover, in the current simple DML case, only
> 3 locks are held till transaction end (target table, virtual XID, XID), and
> we're going to add one more here, which seems questionable to me.
>
> The root issue discussed here is that the relcache is stale due to insufficient
> locking, and the standard approach to fixing such issues is to take a strong
> lock during DDL rather than DML. The publication DDL case is special since it
> needs to lock many tables, but I think we should build on that and improve
> gradually rather than immediately strengthening the lock on the DML side. My
> point is we should first put more effort into engineering the DDL side to solve
> the issue, and if that turns out not to work, then we might consider the
> DML-side approach as a last resort.
>
> Of course, testing is also worthwhile, but I think even if the results show
> nothing, we still need broad consensus if we want to choose that approach.

Thanks, let's proceed with this approach.
I found a few improvements that could be done:
1) Here I felt, we need not call find_all_inheritors for all
relations, we can call it only if the relation is
RELKIND_PARTITIONED_TABLE:
oldrel->relation = table_open(oldrelid,
- ShareUpdateExclusiveLock);
+ ShareRowExclusiveLock);
+ (void) find_all_inheritors(oldrelid, ShareRowExclusiveLock,
+ NULL);

2) Here there is a chance of overflow happening, better to use it like
how NLOCKENTS uses in lock.c:
foreach_oid(schemaid, schemas)
+ relids = list_concat(relids,
+ GetSchemaPublicationRelations(schemaid,
+
PUBLICATION_PART_LEAF));
+
+ maxlocks = (uint64) (max_locks_per_xact * (MaxBackends +
max_prepared_xacts - 1)) / 2;

3) This change is not required:
@@ -2033,6 +2066,7 @@ LockSchemaList(List *schemalist)
}
}
+
/*

The attached v3 version patch has the changes for the same.

Regards,
Vignesh