| From: | Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> |
|---|---|
| To: | Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> |
| Cc: | kedar anavardekar <kedar(dot)anavardekar(at)gmail(dot)com>, shveta malik <shveta(dot)malik(at)gmail(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Ashutosh Sharma <ashu(dot)coek88(at)gmail(dot)com>, "Zhijie Hou (Fujitsu)" <houzj(dot)fnst(at)fujitsu(dot)com>, SATYANARAYANA NARLAPURAM <satyanarlapuram(at)gmail(dot)com>, Fujii Masao <masao(dot)fujii(at)gmail(dot)com>, vignesh C <vignesh21(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: [PATCH] Release replication slot on error in SQL-callable slot functions |
| Date: | 2026-09-20 04:54:00 |
| Message-ID: | CALj2ACVGGutRf=2TA6WX2fP0_B7tugr+U-0Nisny_H-cgzOG_g@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi,
On Fri, Sep 18, 2026 at 11:37 AM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
>
> I reviewed the v17 patch and here are some comments:
Thanks for taking a look at it.
> for use by the current session. Temporary slots are also
> - released upon any error. This function corresponds
> + dropped on any error. An error raised and caught in a
> + subtransaction, for example by a
> + <application>PL/pgSQL</application> exception block, does not
> + drop them. This function corresponds
>
> ISTM what the following sentence says seems to contradict with what
> the first sentence says. How about rephrasing it to:
>
> for use by the current session. Temporary slots are also
> - released upon any error. This function corresponds
> + dropped when an error is reported to the client. An error
> caught inside a
> + subtransaction, for example by a <application>PL/pgSQL</application>
> + exception block, does not drop them. This function corresponds
I think "when an error is reported to the client" may not be apt for
all cases here. For example, a temporary slot could be created and
errored out by a worker internally using SPI interface, not
necessarily by a client. I don't want that confusion, so I dropped the
client part.
> ---
> +-- A slot function that errors out must still release the slot, otherwise the
> +-- next slot operation in the session fails an assertion or leaks the slot.
> +-- Advancing a freshly created slot to a low LSN always errors.
> +SELECT 'init' FROM
> pg_create_logical_replication_slot('regress_subxact_slot',
> 'test_decoding');
>
> The comment seems not to be in the right place; it's in right before
> the pg_create_logical_replication_slot() call but not related. Given
> that we have the comments for subsequent tests, we can remove it.
My intention was to have a comment describing the group of tests, like
the other groups in this test file have. I slightly adjusted it to
match those.
> I've attached the updated patch that incorporated the above comments.
> I'm going to push it early next week, barring any objections.
Please find the attached v18 patches (incl. backbranches) with the
above two slight adjustments. I fixed a typo in the commit message but
otherwise retained it as-is.
--
Bharath Rupireddy
Amazon Web Services: https://aws.amazon.com
| Attachment | Content-Type | Size |
|---|---|---|
| v18-0001-Fix-replication-slot-leak-on-error-caught-in-a-s.patch | application/x-patch | 17.5 KB |
| nocfbot-v18-0001-PG19-Fix-replication-slot-leak-on-error-caught-in-a-s.patch | application/x-patch | 17.5 KB |
| nocfbot-v18-0001-PG18-Fix-replication-slot-leak-on-error-caught-in-a-s.patch | application/x-patch | 17.4 KB |
| nocfbot-v18-0001-PG17-Fix-replication-slot-leak-on-error-caught-in-a-s.patch | application/x-patch | 17.5 KB |
| nocfbot-v18-0001-PG16-Fix-replication-slot-leak-on-error-caught-in-a-s.patch | application/x-patch | 17.6 KB |
| nocfbot-v18-0001-PG15-Fix-replication-slot-leak-on-error-caught-in-a-s.patch | application/x-patch | 17.6 KB |
| nocfbot-v18-0001-PG14-Fix-replication-slot-leak-on-error-caught-in-a-s.patch | application/x-patch | 17.7 KB |
| From | Date | Subject | |
|---|---|---|---|
| Previous Message | Bharath Rupireddy | 2026-09-20 03:52:28 | Re: Add a hook for handling logical decoding messages on subscribers. |