RE: Selectively invalidate caches in pgoutput module
Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com>
From: "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>
To: 'Amit Kapila' <amit.kapila16@gmail.com>
Cc: "pgsql-hackers@lists.postgresql.org" <pgsql-hackers@lists.postgresql.org>, "ShlokKumar.Kyal@fujitsu.com" <ShlokKumar.Kyal@fujitsu.com>, Masahiko Sawada <sawada.mshk@gmail.com>, "Zhijie Hou (Fujitsu)" <houzj.fnst@fujitsu.com>
Date: 2025-03-07T10:58:09Z
Lists: pgsql-hackers
Attachments
- v5-0001-Introduce-a-new-invalidation-message-to-invalidat.patch (application/octet-stream) patch v5-0001
- v5-0002-Invalidate-Relcaches-while-ALTER-PUBLICATION-RENA.patch (application/octet-stream) patch v5-0002
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
Commits
-
Avoid invalidating all RelationSyncCache entries on publication rename.
- 3abe9dc18892 18.0 landed
-
Avoid invalidating all RelationSyncCache entries on publication change.
- 588acf6d0ec1 18.0 landed