On Wed, 14 Oct 2026 at 13:00, Hayato Kuroda (Fujitsu) <[email protected]> wrote: > > Dear Vignesh, > > > The attached v2 version patch has the changes for the same. This also > > addresses hou-san comment of not requiring a new function from [1]. > > Thanks for updating the patch. I've been thinking that > publication_add_relation() > can really cause the cache lookup failure. The caller PublicationAddTables() > acquires the ShareUpdateExclusiveLock for the expressly specified relations > via > OpenTableList(), the issue cannot happen for these targets. So I feel it > might be > OK not to raise an ERROR in this case. > > FYI, I did several experiments, and found that cache lookup failure can happen > with the below schema definition. > > ``` > root // ADD TABLE > ├── mid1 > │ └── victim // to be dropped > └── mid2 > └── trigger > └── other > ``` > > When the parent table is specified by the ALTER PUB command, all > {grand}children > are gathered via find_all_inheritors(NoLock). The cache miss cannot happen if > the > the direct child tables are dropped, because the DROP command will acquire the > AccessExclusive lock. It will conflict with the ALTER PUB command, so these > commands > can be serialized. > > For the grandchild tables, the DROP does not take care the root thus the > concurrent > operation can happen. For reproducing it, find_all_inheritors() must gather > the > OID which is concurrently dropped then invalidate its relcache. Above schema > definition is reachable with the concurrent DROP TABLE victim command. It's > too > complex to provide a reproducer by the injection point though. > > Below briefly shows loops in find_all_inheritors(), and the outer loop can > seek > all relations. If drop is done after the OID is appended to the list and next > iteration checks the pg_inherit in find_inheritance_children(), the relcache > for > victim can be invalidated. > > ``` > foreach(l, rels_list) > { > Oid currentrel = lfirst_oid(l); > List *currentchildren; > ListCell *lc; > > /* Get the direct children of this rel */ > currentchildren = find_inheritance_children(currentrel, lockmode); > // (1) > ... > foreach(lc, currentchildren) > { > ... > rels_list = lappend_oid(rels_list, child_oid); > // (2) > } > > // do DROP TABLE victim; after currentrel points the victim here. > // Must be done after the victim is appended at (2). > // Upcoming (1) acquires a lock for pg_inherits, and a relcache for > victim > // can be invalidated. > } > ``` > > Above meant the cache mis rarely happens. Do we need a guard for the case?
Thanks for reporting this issue, I was able to reproduce this. It was reachable because for a partitioned table OpenTableList() does not lock the partitions (only a non-partitioned inheritance parent gets find_all_inheritors() with ShareUpdateExclusiveLock), so ADD TABLE holds a lock on the root only. The partitions are collected later in GetPubPartitionOptionRelations() with find_all_inheritors(NoLock). Dropping a partition takes AccessExclusiveLock on the partition and on its direct parent, so a direct child conflicts with the ALTER through the root, but a grandchild does not. The updated v3 version patch has the changes to handle the same. Regards, Vignesh
v3-0001-Avoid-cache-lookup-failure-for-concurrently-dropp.patch
Description: Binary data
