From: | "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com> |
---|---|
To: | "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Cc: | "ShlokKumar(dot)Kyal(at)fujitsu(dot)com" <ShlokKumar(dot)Kyal(at)fujitsu(dot)com>, "Zhijie Hou (Fujitsu)" <houzj(dot)fnst(at)fujitsu(dot)com>, 'Amit Kapila' <amit(dot)kapila16(at)gmail(dot)com>, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> |
Subject: | RE: Selectively invalidate caches in pgoutput module |
Date: | 2025-03-11 12:17:14 |
Message-ID: | OSCPR01MB149665015BC2F5DC260E4E184F5D12@OSCPR01MB14966.jpnprd01.prod.outlook.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Dear Amit, Hou,
Thanks for giving comments!
> For patch 0002, I think the implementation could be improved. The
> current patch introduces a new function, RenamePublication, to replace the
> existing generic approach in ExecRenameStmt->AlterObjectRename_internal.
> However, this creates inconsistency because the original code uses
> AccessExclusiveLock for publication renaming, making concurrent renaming
> impossible. The current patch seems to overlook this aspect.
Oh, I missed the point, thanks.
> Additionally, introducing a new function results in code duplication, which can
> be avoided. After further consideration, handling the publication rename
> separately seems unnecessary, given it requires only sending a few extra
> invalidation messages. Therefore, I suggest reusing the generic handling and
> simply extending AlterObjectRename_internal to include the additional
> invalidation messages.
>
> I've attached a diff with the proposed changes for 0002.
Hmm, possible. previously I introduced new function to preserve existing codes as
much as possible. But this introduced some duplicy. Your approach which extends
the common function looks nice, apart from two points;
1. Relsync cache invalidations should be done after the catalog update, but
proposed one did before that. I preferred to add new if-statementfor post catalog-update.
2. Also, common check for the name conflict can be reused.
Attached patch address comments by you and Amit [1]. What's new;
* Skip adding new relsync messages when message for InvalidOid has already exist.
Per comment from Amit [1].
* Update some code comments. Some of them are pointed out by [1].
* Stop using RenamePublication() and extend the common function.
Best regards,
Hayato Kuroda
FUJITSU LIMITED
Attachment | Content-Type | Size |
---|---|---|
v10-0001-Introduce-a-new-invalidation-message-to-invalida.patch | application/octet-stream | 11.3 KB |
v10-0002-Invalidate-Relcaches-while-ALTER-PUBLICATION-REN.patch | application/octet-stream | 4.6 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Junwang Zhao | 2025-03-11 12:19:25 | Re: SQL Property Graph Queries (SQL/PGQ) |
Previous Message | Ashutosh Bapat | 2025-03-11 11:55:56 | Re: Optimizing FastPathTransferRelationLocks() |