RE: Selectively invalidate caches in pgoutput module

From: "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>
To: 'Amit Kapila' <amit(dot)kapila16(at)gmail(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 10:58:09
Message-ID: OSCPR01MB149661B4D4A4E912E8D85CB43F5D52@OSCPR01MB14966.jpnprd01.prod.outlook.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Dear Amit,

Thanks for the comment! PSA new version.

>
> 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?

Actually, I did not find reasons to invalidate even OWNER command for now. I
included it to 1) follow current rule, and to 2) prepare for future update.

1) - Currently RelSyncCache entries are discarded even by ALTER PUBLICATION OWNER
statements. I wanted to preserve it.
2) - In future versions, privilege might be introduced for the publications,
some publications would not be visible for other db users. In this case
I feel we should modify RelationSyncEntry::pubactions, columns and
exprstate thus entries must be discarded.

Now I removed invalidations for OWNER command. Let's revert the change if we miss
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.

Right, fixed.

> 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?

Since the part has been removed from OWNER command, duplicacy was removed.
I did not introduce a function for this.

Best regards,
Hayato Kuroda
FUJITSU LIMITED

Attachment Content-Type Size
v5-0001-Introduce-a-new-invalidation-message-to-invalidat.patch application/octet-stream 8.8 KB
v5-0002-Invalidate-Relcaches-while-ALTER-PUBLICATION-RENA.patch application/octet-stream 6.8 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Bertrand Drouvot 2025-03-07 11:16:07 Re: Log connection establishment timings
Previous Message Nazir Bilal Yavuz 2025-03-07 10:52:09 Re: meson vs. llvm bitcode files