public inbox for [email protected]  

help / color / mirror / Atom feed

From: Tristan Partin <[email protected]>
To: Baji Shaik <[email protected]>
Cc: [email protected]
Cc: [email protected]
Subject: Re: [PATCH] Fix memory leak in pgstat_progress_parallel_incr_param()
Date: Fri, 05 Jun 2026 21:29:46 +0000
Message-ID: <[email protected]> (raw)
In-Reply-To: <CA+fm-RMopta1Dmq8udiU5sp+zwTvhUf4+xfbr3rZDfczH+p-xw@mail.gmail.com>
References: <CA+fm-RMopta1Dmq8udiU5sp+zwTvhUf4+xfbr3rZDfczH+p-xw@mail.gmail.com>

On Fri Jun 5, 2026 at 5:44 PM UTC, Baji Shaik wrote:
> Hi,
>
> While running parallel vacuum with track_cost_delay_timing=on, I
> noticed memory in the parallel worker process keeps growing
> proportionally to vacuum runtime, and is never reclaimed until the
> worker exits.
>
> I think pgstat_progress_parallel_incr_param() (backend_progress.c)
> leaks memory on every call from a parallel worker.
>
> The suspected block:
>
>     static StringInfoData progress_message;
>     initStringInfo(&progress_message);                /* palloc -> A   */
>     pq_beginmessage(&progress_message, PqMsg_Progress);
>     /* pq_beginmessage internally calls initStringInfo again ->
>        palloc -> B, A is orphaned                                       */
>     pq_sendint32(&progress_message, index);
>     pq_sendint64(&progress_message, incr);
>     pq_endmessage(&progress_message);                 /* pfree(B), A leaked
> */
>
> So one palloc(~1 kB) leaks per call, into the per-worker context.
>
> This is an oversight of f1889729dd3 ("Add new parallel message type
> to progress reporting"); track_cost_delay_timing just makes it more
> visible.  With that GUC enabled, a long-running parallel vacuum leaks
> megabytes per worker (~232 MB observed in a 43-min vacuum at default
> settings on a 15M-row, 30-index workload).
>
> The proposed fix is in the attached patch which does a one-time init of the
> static
> buffer, then pq_beginmessage_reuse() / pq_endmessage_reuse() so the
> buffer is allocated once and reused.

Hey Baji,

This looks pretty reasonable to me. Nice find. Did you think about 
keeping the code path as is and just removing the first initStringInfo() 
call? Removing the allocation per progress message seems like a good 
idea to me. Maybe you could separate this change into two patches. One 
to fix the memory leak and another to remove the allocation per message. 
A committer could then decide for themselves if the second patch is 
worth committing.

-- 
Tristan Partin
PostgreSQL Contributors Team
AWS (https://aws.amazon.com)

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]
  Subject: Re: [PATCH] Fix memory leak in pgstat_progress_parallel_incr_param()
  In-Reply-To: <[email protected]>

* 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