Feedback-ID: idd01497b:Fastmail
Mime-Version: 1.0
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain; charset=UTF-8
Date: Fri, 05 Jun 2026 21:29:46 +0000
Message-Id: <DJ1FOVN3MWNT.1ZQ7NHQAVJPXY@partin.io>
Cc: <bertranddrouvot.pg@gmail.com>, <pgsql-hackers@lists.postgresql.org>
Subject: Re: [PATCH] Fix memory leak in
 pgstat_progress_parallel_incr_param()
To: "Baji Shaik" <baji.pgdev@gmail.com>
From: "Tristan Partin" <tristan@partin.io>
References: 
 <CA+fm-RMopta1Dmq8udiU5sp+zwTvhUf4+xfbr3rZDfczH+p-xw@mail.gmail.com>
In-Reply-To: 
 <CA+fm-RMopta1Dmq8udiU5sp+zwTvhUf4+xfbr3rZDfczH+p-xw@mail.gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/DJ1FOVN3MWNT.1ZQ7NHQAVJPXY%40partin.io>
Precedence: bulk

On Fri Jun 5, 2026 at 5:44 PM UTC, Baji Shaik wrote:
> Hi,
>
> While running parallel vacuum with track_cost_delay_timing=3Don, 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 leak=
ed
> */
>
> 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 t=
he
> 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=20
keeping the code path as is and just removing the first initStringInfo()=20
call? Removing the allocation per progress message seems like a good=20
idea to me. Maybe you could separate this change into two patches. One=20
to fix the memory leak and another to remove the allocation per message.=20
A committer could then decide for themselves if the second patch is=20
worth committing.

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