Hi, On Tue, Oct 13, 2026 at 2:55 PM Hayato Kuroda (Fujitsu) <[email protected]> wrote: > > Hi Vignesh, > > > Thanks for reporting this Alexander. > > This fails because RemovePublicationSchemaById() calls > > GetSchemaPublicationRelations(), which collects relation OIDs using a > > catalog snapshot. If a relation is dropped concurrently after that > > snapshot, its OID can still be present in the returned list. > > InvalidatePublicationRels() then calls > > CacheInvalidateRelcacheByRelid() for that OID. By this point, the > > relation may already have been removed and its invalidation processed, > > so the lookup fails with: > > cache lookup failed for relation %u > > To confirm, publication_add_schema()->GetSchemaPublicationRelations() gathers > the > oid of relations without acquiring the lock. Therefore another process can > drop > the relation and this can cause the cache lookup failure, right? > > > I felt InvalidatePublicationRels() should simply skip relations that > > no longer exist. There is no relcache entry left to invalidate, and > > the DROP has already queued the required invalidation. > > I checked your source, but isn't there still a whole that failure can happen? > Relation can be dropped after the SearchSysCache1(). > > How about the below? It tries to open the relation, and skip if cannot. > Relation cache can be invalidated via CacheInvalidateRelcache() instead of > ...ByTuple().
I think it's unnecessary to open the table. IIUC, the main goal is to avoid the cache lookup error in CacheInvalidateRelcacheByRelid(), and the v1 patch is sufficient for that since it uses CacheInvalidateRelcacheByTuple(). The same strategy is also used in RemoveRoleFromObjectPolicy() to handle concurrent table drops. But I think adding a new function for such simple logic is also unnecessary, we can just search the syscache in the loop instead. Best Regards, Zhijie Hou
