MIME-Version: 1.0
References: 
 <CA+fm-RMopta1Dmq8udiU5sp+zwTvhUf4+xfbr3rZDfczH+p-xw@mail.gmail.com>
 <DJ1FOVN3MWNT.1ZQ7NHQAVJPXY@partin.io>
 <CA+fm-RMwUuX7SimgfHZ1o3=RYWQT7wC+KC2+uAgbPqHij25mug@mail.gmail.com>
 <CAA5RZ0sb06bJecz1=0GtM69hO9HarCD_1+m0YOmyZ_PpEs_M6Q@mail.gmail.com>
In-Reply-To: 
 <CAA5RZ0sb06bJecz1=0GtM69hO9HarCD_1+m0YOmyZ_PpEs_M6Q@mail.gmail.com>
From: Baji Shaik <baji.pgdev@gmail.com>
Date: Sat, 6 Jun 2026 11:04:32 -0500
Message-ID: 
 <CA+fm-RNJTNqMFEbk1Fib5i1261PYqJTJBbvNYjyyNqRNnPJjSQ@mail.gmail.com>
Subject: Re: [PATCH] Fix memory leak in pgstat_progress_parallel_incr_param()
To: Sami Imseih <samimseih@gmail.com>
Cc: Tristan Partin <tristan@partin.io>, bertranddrouvot.pg@gmail.com,
	pgsql-hackers@lists.postgresql.org
Content-Type: multipart/mixed; boundary="000000000000a50d53065397ef00"
Archived-At: 
 <https://www.postgresql.org/message-id/CA%2Bfm-RNJTNqMFEbk1Fib5i1261PYqJTJBbvNYjyyNqRNnPJjSQ%40mail.gmail.com>
Precedence: bulk

--000000000000a50d53065397ef00
Content-Type: multipart/alternative; boundary="000000000000a50d51065397effe"

--000000000000a50d51065397effe
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Sat, Jun 6, 2026 at 9:55=E2=80=AFAM Sami Imseih <samimseih@gmail.com> wr=
ote:

> >   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.

--000000000000a50d51065397effe
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr"><div dir=3D"ltr">On Sat, Jun 6, 2026 at 9:55=E2=80=AFAM Sa=
mi Imseih &lt;<a href=3D"mailto:samimseih@gmail.com">samimseih@gmail.com</a=
>&gt; wrote:</div><div class=3D"gmail_quote gmail_quote_container"><blockqu=
ote class=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-left:1px=
 solid rgb(204,204,204);padding-left:1ex">
&gt;=C2=A0 =C2=A00002: allocate the static buffer once per process via<br>
&gt;=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0pq_beginmessage_reuse / pq_endmessage=
_reuse, to avoid the<br>
&gt;=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0per-call allocation (master only)<br>
<br>
I am less convinced this will have any benefits for the additional complexi=
ty.<br>
The callers of pgstat_progress_parallel_incr_param() are not frequent enoug=
h<br>
to make a measurable difference here. cost delay reporting for parallel wor=
kers<br>
is throttled by PARALLEL_VACUUM_DELAY_REPORT_INTERVAL_NS and<br>
index progress reporting does not happen very frequently either.</blockquot=
e><div><br></div>Thanks for the review, Sami.<br><br>Agreed, at those call =
frequencies the per-call palloc is not worth<br><div>the added complexity. =
Dropping 0002.=C2=A0<br><br>v3 attached (just 0001, unchanged from previous=
).</div><div><br></div><div>Thanks,<br>Baji Shaik.</div></div></div>

--000000000000a50d51065397effe--

--000000000000a50d53065397ef00
Content-Type: application/octet-stream;
	name="v3-0001-Fix-memory-leak-in-pgstat_progress_parallel_incr_.patch"
Content-Disposition: attachment;
	filename="v3-0001-Fix-memory-leak-in-pgstat_progress_parallel_incr_.patch"
Content-Transfer-Encoding: base64
Content-ID: <f_mq2jk46x0>
X-Attachment-Id: f_mq2jk46x0

RnJvbSA1MDdmNGZjOTAzMGJjYjQ3MDhmZjI5ZDA0ZmNkNzE1NDYzMzk2Y2NiIE1vbiBTZXAgMTcg
MDA6MDA6MDAgMjAwMQpGcm9tOiBCYWppIFNoYWlrIDxiYWppLnBnZGV2QGdtYWlsLmNvbT4KRGF0
ZTogU2F0LCA2IEp1biAyMDI2IDEwOjU3OjU0IC0wNTAwClN1YmplY3Q6IFtQQVRDSCB2M10gRml4
IG1lbW9yeSBsZWFrIGluIHBnc3RhdF9wcm9ncmVzc19wYXJhbGxlbF9pbmNyX3BhcmFtKCkKCldo
ZW4gY2FsbGVkIGZyb20gYSBwYXJhbGxlbCB3b3JrZXIsIHBnc3RhdF9wcm9ncmVzc19wYXJhbGxl
bF9pbmNyX3BhcmFtKCkKY2FsbHMgaW5pdFN0cmluZ0luZm8oKSBvbiBhIHN0YXRpYyBTdHJpbmdJ
bmZvRGF0YSBhbmQgdGhlbiBpbW1lZGlhdGVseQpjYWxscyBwcV9iZWdpbm1lc3NhZ2UoKSwgd2hp
Y2ggY2FsbHMgaW5pdFN0cmluZ0luZm8oKSBhZ2Fpbi4gIFRoZSBzZWNvbmQKY2FsbCBvdmVyd3Jp
dGVzIGJ1Zi0+ZGF0YSB3aXRoIGEgZnJlc2hseSBwYWxsb2MnZCBidWZmZXIsIG9ycGhhbmluZyB0
aGUKZmlyc3Qgb25lLiAgcHFfZW5kbWVzc2FnZSgpIHRoZW4gZnJlZXMgb25seSB0aGUgc2Vjb25k
IGJ1ZmZlciwgc28gZWFjaApjYWxsIGxlYWtzIH4xIGtCIGludG8gdGhlIHBlci13b3JrZXIgbWVt
b3J5IGNvbnRleHQuCgpGaXggYnkgcmVtb3ZpbmcgdGhlIHJlZHVuZGFudCBpbml0U3RyaW5nSW5m
bygpIGNhbGwuCgpPdmVyc2lnaHQgb2YgZjE4ODk3MjlkZDMgKCJBZGQgbmV3IHBhcmFsbGVsIG1l
c3NhZ2UgdHlwZSB0byBwcm9ncmVzcwpyZXBvcnRpbmciKS4KCkF1dGhvcjogQmFqaSBTaGFpayA8
YmFqaS5wZ2RldkBnbWFpbC5jb20+Ci0tLQogc3JjL2JhY2tlbmQvdXRpbHMvYWN0aXZpdHkvYmFj
a2VuZF9wcm9ncmVzcy5jIHwgMiAtLQogMSBmaWxlIGNoYW5nZWQsIDIgZGVsZXRpb25zKC0pCgpk
aWZmIC0tZ2l0IGEvc3JjL2JhY2tlbmQvdXRpbHMvYWN0aXZpdHkvYmFja2VuZF9wcm9ncmVzcy5j
IGIvc3JjL2JhY2tlbmQvdXRpbHMvYWN0aXZpdHkvYmFja2VuZF9wcm9ncmVzcy5jCmluZGV4IGIw
MzU5NzcxZGU1Li42ZDIwNDkxMDVhYiAxMDA2NDQKLS0tIGEvc3JjL2JhY2tlbmQvdXRpbHMvYWN0
aXZpdHkvYmFja2VuZF9wcm9ncmVzcy5jCisrKyBiL3NyYy9iYWNrZW5kL3V0aWxzL2FjdGl2aXR5
L2JhY2tlbmRfcHJvZ3Jlc3MuYwpAQCAtMTAwLDggKzEwMCw2IEBAIHBnc3RhdF9wcm9ncmVzc19w
YXJhbGxlbF9pbmNyX3BhcmFtKGludCBpbmRleCwgaW50NjQgaW5jcikKIAl7CiAJCXN0YXRpYyBT
dHJpbmdJbmZvRGF0YSBwcm9ncmVzc19tZXNzYWdlOwogCi0JCWluaXRTdHJpbmdJbmZvKCZwcm9n
cmVzc19tZXNzYWdlKTsKLQogCQlwcV9iZWdpbm1lc3NhZ2UoJnByb2dyZXNzX21lc3NhZ2UsIFBx
TXNnX1Byb2dyZXNzKTsKIAkJcHFfc2VuZGludDMyKCZwcm9ncmVzc19tZXNzYWdlLCBpbmRleCk7
CiAJCXBxX3NlbmRpbnQ2NCgmcHJvZ3Jlc3NfbWVzc2FnZSwgaW5jcik7Ci0tIAoyLjUwLjEgKEFw
cGxlIEdpdC0xNTUpCgo=
--000000000000a50d53065397ef00--