On Tue, 6 Oct 2026 at 17:50, Zhijie Hou <[email protected]> wrote: > > Hi, > > On Tue, Oct 6, 2026 at 5:23 PM shveta malik <[email protected]> wrote: > > > > On Sun, Oct 4, 2026 at 5:56 PM Zhijie Hou <[email protected]> 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
v3-0001-Lock-tables-against-writers-when-altering-publica.patch
Description: Binary data
v3-0002-Synchronize-publication-scope-widening-with.patch
Description: Binary data
