Re: pg_walinspect memory leaks - Mailing list pgsql-hackers

From Jeff Davis
Subject Re: pg_walinspect memory leaks
Date
Msg-id c3935d2a3c018f21e57fffcd0935e38c201629dc.camel@j-davis.com
Whole thread Raw
In response to Re: pg_walinspect memory leaks  (Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>)
Responses Re: pg_walinspect memory leaks
List pgsql-hackers
On Mon, 2023-02-20 at 15:17 +0530, Bharath Rupireddy wrote:

> Similarly, the loops in GetXLogSummaryStats() too
> don't palloc any memory, so no memory leak.

Break on palloc in gdb in that loop and you'll see a palloc in
CStringGetTextDatum(name). In general, you should expect *GetDatum() to
palloc unless you're sure that it's pass-by-value. Even
Float8GetDatum() has code to account for pass-by-ref float8s.

There are also a couple calls to psprintf() in the stats_per_record
path.

>  I've seen no memory growth
> during execution of pg_get_wal_stats_till_end_of_wal() for 35million
> WAL records, see [1] PID 543967 (during the execution of the stats
> function, the memory usage remained constant). Therefore, I feel that
> the fix isn't required for GetWalStats().

That is true because the loops in GetXLogSummaryStats() are based on
constants. It does at most RM_MAX_ID * MAX_XLINFO_TYPES calls to
FillXLogStatsRow() regardless of the number of WAL records.
It's not a significant amount of memory, at least today. But, since
we're already using the temp context pattern, we might as well use it
here for clarity so that we don't have to guess about whether the
amount of memory is significant or not.

Committed to 16 with the changes to GetXLogSummaryStats() as well.
Committed unmodified version of your 15 backport. Thank you!


--
Jeff Davis
PostgreSQL Contributor Team - AWS





pgsql-hackers by date:

Previous
From: David Rowley
Date:
Subject: Re: Reducing System Allocator Thrashing of ExecutorState to Alleviate FDW-related Performance Degradations
Next
From: Daniel Gustafsson
Date:
Subject: Re: meson: Non-feature feature options