Subject: pgsql: Avoid invalidating all RelationSyncCache entries on publication

From: Amit Kapila <[email-masked]>
Date: Thu, 13 Mar 2025 04:00:15 +0000

Avoid invalidating all RelationSyncCache entries on publication rename.

On Publication rename, we need to only invalidate the RelationSyncCache
entries corresponding to relations that are part of the publication being
renamed.

As part of this patch, we introduce a new invalidation message to
invalidate the cache maintained by the logical decoding output plugin. We
can't use existing relcache invalidation for this purpose, as that would
unnecessarily cause relcache invalidations in other backends.

This will improve performance by building fewer relation cache entries
during logical replication.

Author: Hayato Kuroda <[email-masked]>
Author: Shlok Kyal <[email-masked]>
Reviewed-by: Hou Zhijie <[email-masked]>
Reviewed-by: Amit Kapila <[email-masked]>
Discussion: https://postgr.es/m/[email-masked]

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/3abe9dc18892b9f69bb48a2eb21fbe5cf348a489

Modified Files
--------------
src/backend/access/rmgrdesc/standbydesc.c   |   2 +
src/backend/commands/alter.c                |  16 ++++
src/backend/commands/publicationcmds.c      |  39 +++++++++
src/backend/replication/pgoutput/pgoutput.c |   8 +-
src/backend/utils/cache/inval.c             | 125 ++++++++++++++++++++++++++++
src/include/commands/publicationcmds.h      |   1 +
src/include/pg_config_manual.h              |   8 +-
src/include/storage/sinval.h                |  24 ++++--
src/include/utils/inval.h                   |  10 +++
src/test/subscription/t/007_ddl.pl          |  85 +++++++++++++++++++
10 files changed, 302 insertions(+), 16 deletions(-)

---

From: Robert Haas <[email-masked]>
Date: Mon, 17 Mar 2025 12:05:22 -0400

> unnecessarily cause relcache invalidations in other backends.

This seems like too much infrastructure for a niche optimization. If
there are plans to use this new invalidation message type to optimize
a bunch of other cases, then maybe it's worth it, but adding this just
to cover the presumably-rare case of ALTER PUBLICATION .. RENAME
doesn't seem worth it to me.

-- 
Robert Haas
EDB: http://www.enterprisedb.com

---

From: Amit Kapila <[email-masked]>
Date: Tue, 18 Mar 2025 12:00:04 +0530

> doesn't seem worth it to me.

This commit will be helpful for many other existing commands like:
CREATE PUBLICATION ...
ALTER PUBLICATION name SET (publication_parameter)
ALTER PUBLICATION name OWNER ...
DROP PUBLICATION ...

Before this, commit any publication command that modified
pg_publication catalog use to invalidate the entire RelationSyncCache.
The code that led to entire RelationSyncCache invalidation was removed
in this commit:
publication_invalidation_cb(Datum arg, int cacheid, uint32 hashvalue)
 {
    publications_valid = false;
-
-   /*
-    * Also invalidate per-relation cache so that next time the filtering info
-    * is checked it will be updated with the new publication settings.
-    */
-   rel_sync_cache_publication_cb(arg, cacheid, hashvalue);
 }

We can't remove this without having a solution for ALTER PUBLICATION
.. RENAME command to invalidate the specific RelationSyncCache
entries. As mentioned in the commit message, the other idea was to
register a relcache invalidation for all relations that are part of a
publication which we felt could be harmful to other backends that are
not even involved in decoding. Now, the ALTER PUBLICATION name OWNER
command doesn't need to use this new invalidation at this stage
because it doesn't impact the RelationSyncCache entries but it
benefits from not requiring to invalidate the entire cache. Similarly,
the other commands mentioned above uses relcache invalidation for this
purpose but the difference is that the other commands do need relcache
invalidation for the purpose of correctness whereas ALTER PUBLICATION
.. RENAME command doesn't need it.

In the future, we can use this invalidation in existing publication
commands like altering a publication where we only publish 'INSERT'
(and or 'TRUNCATE') and change other publication properties like
'publish_via_partition_root', 'publish_generated_columns'. Similarly,
I think we can even use new invalidation for SET/ADD/DROP variants of
Alter PUBLICATION where the publication publishes only 'INSERT' and or
'TRUNCATE'. Before this commit, we didn't have a better way so we used
relcache invalidations for these cases as well. Also, any new
publication property or command should prefer to use this new
invalidation instead of using a relcache invalidation unless required
the same for correctness.

The impact of not doing any solution for this problem (aka removing
the above code in publication_invalidation_cb) could be magnified in
future commits where we are trying to solve a long-standing data loss
issue in logical decoding/replication via distributing invalidations
to all in-progress transactions [1]. You can read the commit message
to understand that problem and solution.

I think I should have mentioned the future use and other improvements
of this work in the commit message.

[1] - https://www.postgresql.org/message-id/OSCPR01MB14966DDB92FA7DA8FA8658216F5DE2%40OSCPR01MB14966.jpnprd01.prod.outlook.com

-- 
With Regards,
Amit Kapila.

---

From: Robert Haas <[email-masked]>
Date: Tue, 18 Mar 2025 09:42:38 -0400

> of this work in the commit message.

OK, thanks for the background. Yes, it would have been helpful to
mention at least in general terms that it would unblock other
optimization work.