Re: Why our Valgrind reports suck - Mailing list pgsql-hackers

From Yasir
Subject Re: Why our Valgrind reports suck
Date
Msg-id CAA9OW9dqJHmY1-tPpZUM6-w5nfYChD4160bTsr7dLki7Gk0dpA@mail.gmail.com
Whole thread Raw
List pgsql-hackers


On Wed, May 21, 2025 at 10:14 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Here's a v2 patchset that reaches the goal of zero reported leaks
in the core regression tests, with some caveats:

* Rather than completely fixing the function-cache and
TS-dictionary-cache issues, I just added suppression rules to
hide them.  I am not convinced it is worth working harder than that.
The patchset does include some fixes that clean up low-hanging fruit
in that area, but going further seems like a lot of work (and risk of
bugs) for fairly minimal gain.  The core regression tests show less
than 10K "suppressed" space in all test sessions but three, and those
three are still under 100K.

* The patch series assumes that the ModifyTable fix discussed at [1]
is already applied.

* I still observe leaks in ProcessGetMemoryContextInterrupt, but
I think the consensus is we should just revert that as not yet ready
for prime time [2].

0001 is the same as before except I did more work on the comments.
I concluded that we were overloading the term "chunk" too much,
so I invented the term "vchunk" for Valgrind's notion of chunks.
(Feel free to suggest other terminology.)

0002 is new work to fix up MemoryContextAllocAligned so it doesn't
cause possible-leak complaints.


I tested with the provided v2 patch series making sure mentioned [1] applied. 
More than 800 backend valgrind output files generated against regression, among which 237 files contain suppressed: > 0 entries, of which 5 files also contain "definitely lost: > 0 bytes entries". 
The Maximum leak I found in these valgrind output files is 960 bytes only.

Whilst, the original issue I posted [link] is fixed. There are no leaks. 

Regards, 

Yasir Hussain
Data Bene
 
The rest are more or less bite-sized fixes of individual problems.
Probably we could squash a lot of them for final commit, but I
thought it'd be easier to review like this.  Note that I'm not
expecting 0013 to get applied in this form [3], but without it we
have various gripes about memory leaked from plancache entries.

                        regards, tom lane

[1] https://www.postgresql.org/message-id/flat/213261.1747611093%40sss.pgh.pa.us
[2] https://www.postgresql.org/message-id/594293.1747708165%40sss.pgh.pa.us
[3] https://www.postgresql.org/message-id/605328.1747710381%40sss.pgh.pa.us

pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: [Util] Warn and Remove Invalid GUCs
Next
From: Jim Jones
Date:
Subject: Re: [PATCH] Add pretty-printed XML output option