RE: Selectively invalidate caches in pgoutput module

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

In response to

Responses

Browse pgsql-hackers by date

  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()