Re: Test publication.sql racing with subscription.sql might fail due to cache lookup error

From: vignesh C <vignesh21(at)gmail(dot)com>
To: Zhijie Hou <houzhijie22(at)gmail(dot)com>
Cc: "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>, Alexander Lakhin <exclusion(at)gmail(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Test publication.sql racing with subscription.sql might fail due to cache lookup error
Date: 2026-10-13 07:41:16
Message-ID: CALDaNm3AhSPWT5+qCwsgtPykAbNJK4ZroXu25EreFvNxoYu0vw@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, 13 Oct 2026 at 12:55, Zhijie Hou <houzhijie22(at)gmail(dot)com> wrote:
>
> Hi,
>
> On Tue, Oct 13, 2026 at 2:55 PM Hayato Kuroda (Fujitsu)
> <kuroda(dot)hayato(at)fujitsu(dot)com> 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.

I think the same issue can occur in other callers of
InvalidatePublicationRels() apart from the reported adding tables in a
schema, such as altering publication options, dropping a table from a
publication, and dropping a schema from a publication. We can safely
ignore relations that have been dropped concurrently in these cases.
The other caller, which adds a table directly to a publication, is not
affected because it locks the table and prevents it from being dropped
concurrently. So, we can continue to throw an error if the table no
longer exists in that case.
Thoughts?

Regards,
Vignesh

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Fujii Masao 2026-10-13 07:54:51 Re: [PG19] psql backslash commands fail with translated messages and a non UTF-8 client encoding
Previous Message Zhijie Hou 2026-10-13 07:25:16 Re: Test publication.sql racing with subscription.sql might fail due to cache lookup error