public inbox for [email protected]
help / color / mirror / Atom feedFrom: Baji Shaik <[email protected]>
To: Sami Imseih <[email protected]>
Cc: Tristan Partin <[email protected]>
Cc: [email protected]
Cc: [email protected]
Subject: Re: [PATCH] Fix memory leak in pgstat_progress_parallel_incr_param()
Date: Sat, 6 Jun 2026 11:04:32 -0500
Message-ID: <CA+fm-RNJTNqMFEbk1Fib5i1261PYqJTJBbvNYjyyNqRNnPJjSQ@mail.gmail.com> (raw)
In-Reply-To: <CAA5RZ0sb06bJecz1=0GtM69hO9HarCD_1+m0YOmyZ_PpEs_M6Q@mail.gmail.com>
References: <CA+fm-RMopta1Dmq8udiU5sp+zwTvhUf4+xfbr3rZDfczH+p-xw@mail.gmail.com>
<[email protected]>
<CA+fm-RMwUuX7SimgfHZ1o3=RYWQT7wC+KC2+uAgbPqHij25mug@mail.gmail.com>
<CAA5RZ0sb06bJecz1=0GtM69hO9HarCD_1+m0YOmyZ_PpEs_M6Q@mail.gmail.com>
On Sat, Jun 6, 2026 at 9:55 AM Sami Imseih <[email protected]> wrote:
> > 0002: allocate the static buffer once per process via
> > pq_beginmessage_reuse / pq_endmessage_reuse, to avoid the
> > per-call allocation (master only)
>
> I am less convinced this will have any benefits for the additional
> complexity.
> The callers of pgstat_progress_parallel_incr_param() are not frequent
> enough
> to make a measurable difference here. cost delay reporting for parallel
> workers
> is throttled by PARALLEL_VACUUM_DELAY_REPORT_INTERVAL_NS and
> index progress reporting does not happen very frequently either.
Thanks for the review, Sami.
Agreed, at those call frequencies the per-call palloc is not worth
the added complexity. Dropping 0002.
v3 attached (just 0001, unchanged from previous).
Thanks,
Baji Shaik.
Attachments:
[application/octet-stream] v3-0001-Fix-memory-leak-in-pgstat_progress_parallel_incr_.patch (1.5K, 3-v3-0001-Fix-memory-leak-in-pgstat_progress_parallel_incr_.patch)
download | inline diff:
From 507f4fc9030bcb4708ff29d04fcd715463396ccb Mon Sep 17 00:00:00 2001
From: Baji Shaik <[email protected]>
Date: Sat, 6 Jun 2026 10:57:54 -0500
Subject: [PATCH v3] Fix memory leak in pgstat_progress_parallel_incr_param()
When called from a parallel worker, pgstat_progress_parallel_incr_param()
calls initStringInfo() on a static StringInfoData and then immediately
calls pq_beginmessage(), which calls initStringInfo() again. The second
call overwrites buf->data with a freshly palloc'd buffer, orphaning the
first one. pq_endmessage() then frees only the second buffer, so each
call leaks ~1 kB into the per-worker memory context.
Fix by removing the redundant initStringInfo() call.
Oversight of f1889729dd3 ("Add new parallel message type to progress
reporting").
Author: Baji Shaik <[email protected]>
---
src/backend/utils/activity/backend_progress.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/src/backend/utils/activity/backend_progress.c b/src/backend/utils/activity/backend_progress.c
index b0359771de5..6d2049105ab 100644
--- a/src/backend/utils/activity/backend_progress.c
+++ b/src/backend/utils/activity/backend_progress.c
@@ -100,8 +100,6 @@ pgstat_progress_parallel_incr_param(int index, int64 incr)
{
static StringInfoData progress_message;
- initStringInfo(&progress_message);
-
pq_beginmessage(&progress_message, PqMsg_Progress);
pq_sendint32(&progress_message, index);
pq_sendint64(&progress_message, incr);
--
2.50.1 (Apple Git-155)
view thread (6+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: [PATCH] Fix memory leak in pgstat_progress_parallel_incr_param()
In-Reply-To: <CA+fm-RNJTNqMFEbk1Fib5i1261PYqJTJBbvNYjyyNqRNnPJjSQ@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox