| From: | shveta malik <shveta(dot)malik(at)gmail(dot)com> |
|---|---|
| To: | "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com> |
| Cc: | Nisha Moond <nisha(dot)moond412(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, "Zhijie Hou (Fujitsu)" <houzj(dot)fnst(at)fujitsu(dot)com>, shveta malik <shveta(dot)malik(at)gmail(dot)com> |
| Subject: | Re: Fix resource leak in FindConflictTuple() retry path |
| Date: | 2026-09-03 09:06:33 |
| Message-ID: | CAJpy0uArvvA1ut26qhqR6=Tf6RrEHiJKd9155Ce2LMNWNwvbCg@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Thu, Sep 3, 2026 at 1:03 PM Hayato Kuroda (Fujitsu)
<kuroda(dot)hayato(at)fujitsu(dot)com> wrote:
>
> Dear Nisha,
>
> Not sure, should we release a buffer pin if should_refetch_tuple() returns true?
> I referred heapam_tuple_lock()/heap_lock_tuple(), they pin a buffer via ReadBuffer()
> and transfers to a slot via ExecStorePinnedBufferHeapTuple().
>
> IIUC, ExecClearTuple() can release corresponding resources for the slot, so below
> fix is enough.
>
IMO, ExecClearTuple() can resolve the problem of pinned buffers but it
still leaves the other trivial problem behind that we keep allocating
slots while one is enough, which when resued will release buffer pin
in ExecStorePinnedBufferHeapTuple() -> tts_buffer_heap_store_tuple()
implicitly.
Therefore, this alone might not be a better fix. But if we want to
club it with the fix already provided by Nisha, that will help us
releasing the pin a little earlier without relying on next
table_tuple_lock() on same slot to do that implictly. That said, I
don't see ExecClearTuple() called explicitly in the two similar
functions in this file, RelationFindReplTupleByIndex() and
RelationFindReplTupleSeq(); both of which reuse a single
caller-provided slot across retries and rely purely on the implicit
release-on-restore behavior.
> ```
> --- a/src/backend/executor/execReplication.c
> +++ b/src/backend/executor/execReplication.c
> @@ -268,7 +268,10 @@ retry:
> PopActiveSnapshot();
>
> if (should_refetch_tuple(res, &tmfd))
> + {
> + ExecClearTuple(*conflictslot);
> goto retry;
> + }
> ```
>
> Best regards,
> Hayato Kuroda
> FUJITSU LIMITED
>
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Shlok Kyal | 2026-09-03 09:09:07 | Re: logical decoding: skip unnecessary snapshot distribution. |
| Previous Message | Christoph Berg | 2026-09-03 08:42:34 | Re: pgsql: Fix compilation with OpenSSL 4 |