public inbox for [email protected]help / color / mirror / Atom feed
pgsql: Avoid invalidating all RelationSyncCache entries on publication 5+ messages / 3 participants [nested] [flat]
* pgsql: Avoid invalidating all RelationSyncCache entries on publication @ 2025-03-06 08:55 Amit Kapila <[email protected]> 0 siblings, 0 replies; 5+ messages in thread From: Amit Kapila @ 2025-03-06 08:55 UTC (permalink / raw) To: [email protected] Avoid invalidating all RelationSyncCache entries on publication change. On change of publication via ALTER PUBLICATION ... SET/ADD/DROP commands, we were invalidating all the relations present in relation sync cache maintained by pgoutput. We need to invalidate only the relation entries that are changed as part of publication DDL. We have ensured that the publication DDL execution generated the invalidations required to invalidate impacted relation sync entries in RelationSyncCache. This improves the performance by avoiding building the cache entries for the cases where a publication has many tables but only one of them is dropped. Author: Shlok Kyal <[email protected]> Author: Hayato Kuroda <[email protected]> Reviewed-by: Hou Zhijie <[email protected]> Reviewed-by: Amit Kapila <[email protected]> Discussion: https://postgr.es/m/OSCPR01MB14966C09AA201EFFA706576A7F5C92@OSCPR01MB14966.jpnprd01.prod.outlook.com Branch ------ master Details ------- https://git.postgresql.org/pg/commitdiff/588acf6d0ec15d9384c2f712585c0f56936d7bac Modified Files -------------- src/backend/replication/pgoutput/pgoutput.c | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-) ^ permalink raw reply [nested|flat] 5+ messages in thread
* pgsql: Avoid invalidating all RelationSyncCache entries on publication @ 2025-03-13 04:00 Amit Kapila <[email protected]> 0 siblings, 1 reply; 5+ messages in thread From: Amit Kapila @ 2025-03-13 04:00 UTC (permalink / raw) To: [email protected] 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 protected]> Author: Shlok Kyal <[email protected]> Reviewed-by: Hou Zhijie <[email protected]> Reviewed-by: Amit Kapila <[email protected]> Discussion: https://postgr.es/m/OSCPR01MB14966C09AA201EFFA706576A7F5C92@OSCPR01MB14966.jpnprd01.prod.outlook.com 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(-) ^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: pgsql: Avoid invalidating all RelationSyncCache entries on publication @ 2025-03-17 16:05 Robert Haas <[email protected]> parent: Amit Kapila <[email protected]> 0 siblings, 1 reply; 5+ messages in thread From: Robert Haas @ 2025-03-17 16:05 UTC (permalink / raw) To: Amit Kapila <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]> On Thu, Mar 13, 2025 at 12:00 AM Amit Kapila <[email protected]> wrote: > 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 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 ^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: pgsql: Avoid invalidating all RelationSyncCache entries on publication @ 2025-03-18 06:30 Amit Kapila <[email protected]> parent: Robert Haas <[email protected]> 0 siblings, 1 reply; 5+ messages in thread From: Amit Kapila @ 2025-03-18 06:30 UTC (permalink / raw) To: Robert Haas <[email protected]>; +Cc: Amit Kapila <[email protected]>; PostgreSQL Hackers <[email protected]> On Mon, Mar 17, 2025 at 9:35 PM Robert Haas <[email protected]> wrote: > > On Thu, Mar 13, 2025 at 12:00 AM Amit Kapila <[email protected]> wrote: > > 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 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. > 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.jpnpr... -- With Regards, Amit Kapila. ^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: pgsql: Avoid invalidating all RelationSyncCache entries on publication @ 2025-03-18 13:42 Robert Haas <[email protected]> parent: Amit Kapila <[email protected]> 0 siblings, 0 replies; 5+ messages in thread From: Robert Haas @ 2025-03-18 13:42 UTC (permalink / raw) To: Amit Kapila <[email protected]>; +Cc: Amit Kapila <[email protected]>; PostgreSQL Hackers <[email protected]> On Tue, Mar 18, 2025 at 2:30 AM Amit Kapila <[email protected]> wrote: > I think I should have mentioned the future use and other improvements > 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. -- Robert Haas EDB: http://www.enterprisedb.com ^ permalink raw reply [nested|flat] 5+ messages in thread
end of thread, other threads:[~2025-03-18 13:42 UTC | newest] Thread overview: 5+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2025-03-06 08:55 pgsql: Avoid invalidating all RelationSyncCache entries on publication Amit Kapila <[email protected]> 2025-03-13 04:00 pgsql: Avoid invalidating all RelationSyncCache entries on publication Amit Kapila <[email protected]> 2025-03-17 16:05 ` Robert Haas <[email protected]> 2025-03-18 06:30 ` Amit Kapila <[email protected]> 2025-03-18 13:42 ` Robert Haas <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox