Re: Selectively invalidate caches in pgoutput module

From: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
To: "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>
Cc: "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org>, "ShlokKumar(dot)Kyal(at)fujitsu(dot)com" <ShlokKumar(dot)Kyal(at)fujitsu(dot)com>, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, "Zhijie Hou (Fujitsu)" <houzj(dot)fnst(at)fujitsu(dot)com>
Subject: Re: Selectively invalidate caches in pgoutput module
Date: 2025-03-07 08:12:20
Message-ID: CAA4eK1LMR7anY0Rv7sUFYFcqxetzV-M9BVsQYAm=OAp=3nL4CQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Mar 6, 2025 at 5:16 PM Hayato Kuroda (Fujitsu)
<kuroda(dot)hayato(at)fujitsu(dot)com> wrote:
>
> Dear Amit, hackers,
>
> > Yeah, this is a good improvement and the patch looks safe to me, so I
> > pushed with minor changes in the comments.
>
> Thanks! PSA rebased patch.
>

Few comments:
1. Why do we need to invalidate relsync entries when owner of its
publication changes?

I think the owner change will impact the future Alter Publication ...
Add/Drop/Set/Rename operations as that will be allowed only to new
owner (or super users), otherwise, there shouldn't be an impact on
RelSyncCache entries. Am, I missing something?

2.
+ if (pubform->puballtables)
+ {
+ CacheInvalidateRelSyncAll();
+ }
+ else
+ {
+ List *relids = NIL;
+ List *schemarelids = NIL;
+
+ /*
+ * For partition table, when we insert data, get_rel_sync_entry is
+ * called and a hash entry is created for the corresponding leaf table.
+ * So invalidating the leaf nodes would be sufficient here.
+ */
+ relids = GetPublicationRelations(pubform->oid,
+ PUBLICATION_PART_LEAF);
+ schemarelids = GetAllSchemaPublicationRelations(pubform->oid,
+ PUBLICATION_PART_LEAF);
+
+ relids = list_concat_unique_oid(relids, schemarelids);
+
+ InvalidateRelSyncCaches(relids);
+ }
+
+ CatalogTupleUpdate(rel, &tup->t_self, tup);

Shouldn't we need to update the CatalogTuple before invalidations.

3.
+ if (pubform->puballtables)
+ {
+ CacheInvalidateRelSyncAll();
+ }
+ else
+ {
+ List *relids = NIL;
+ List *schemarelids = NIL;
+
+ /*
+ * For partition table, when we insert data, get_rel_sync_entry is
+ * called and a hash entry is created for the corresponding leaf table.
+ * So invalidating the leaf nodes would be sufficient here.
+ */
+ relids = GetPublicationRelations(pubform->oid,
+ PUBLICATION_PART_LEAF);
+ schemarelids = GetAllSchemaPublicationRelations(pubform->oid,
+ PUBLICATION_PART_LEAF);
+
+ relids = list_concat_unique_oid(relids, schemarelids);
+
+ InvalidateRelSyncCaches(relids);
+ }

This is a duplicate code. Can we write a function to eliminate this duplicacy?

--
With Regards,
Amit Kapila.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Bertrand Drouvot 2025-03-07 08:33:04 Re: per backend WAL statistics
Previous Message Peter Eisentraut 2025-03-07 08:09:18 Re: what's going on with lapwing?