| From: | Michael Paquier <michael(at)paquier(dot)xyz> |
|---|---|
| To: | Sami Imseih <samimseih(at)gmail(dot)com> |
| Cc: | Baji Shaik <baji(dot)pgdev(at)gmail(dot)com>, Tristan Partin <tristan(at)partin(dot)io>, bertranddrouvot(dot)pg(at)gmail(dot)com, pgsql-hackers(at)lists(dot)postgresql(dot)org |
| Subject: | Re: [PATCH] Fix memory leak in pgstat_progress_parallel_incr_param() |
| Date: | 2026-06-08 03:44:05 |
| Message-ID: | aiY6hfUWWU0Qi24t@paquier.xyz |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Sat, Jun 06, 2026 at 09:55:05AM -0500, Sami Imseih wrote:
> Hi,
>
> good find, and thanks for the patches!
>
> > 0001: drop the redundant initStringInfo() call (backport candidate)
>
> This one looks like an obvious fix to me.
And clearly something that should be backpatched down to v17, or we
could pile a lot of memory depending on how many calls we do in a
worker, with more piling over time.
Will process, thanks!
>> 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.
I doubt that 0002 is worth doing, particularly seeing the code paths
where this is called.
--
Michael
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Chao Li | 2026-06-08 03:53:29 | Fix ALTER DOMAIN VALIDATE CONSTRAINT locking |
| Previous Message | Bruce Momjian | 2026-06-08 03:43:16 | Re: First draft of PG 19 release notes |