> On Wed, Apr 08, 2026 at 04:33:00PM -0500, Sami Imseih wrote:
> >> On Wed, Apr 08, 2026 at 04:40:03PM -0400, Andres Freund wrote:
> >> > Note that the whole cached state does automatically get reset at the end of
> >> > the transaction (AtEOXact_PgStat()->pgstat_clear_snapshot()), just like it did
> >> > before the shmem stats stuff.
> >>
> >> I see a lot of memory used for the pgStatEntryRefHash table, too (e.g., ~16
> >> MB for 100K tables). What's interesting is that I cannot reproduce similar
> >> usage with views like pg_stat_all_tables. If memory was not a concern, I
> >> think the "bool *may_free" idea would be fine.
> >
> > Instead of may_free, which is invasive, what about pgstat_fetch_entry_nocache
> > which can be called by 2 new APIs pgstat_fetch_stat_tabentry_nocache() and
> > pgstat_fetch_stat_tabentry_nocache_ext(). This way a caller that uses
> > these will be required to pfree?
>
> This might help avoid memory usage within a snapshot,
Yes, this is exactly why it would be useful for autovacuum workers and
the scores view. relation_needs_vacanalyze() can use the nocache variant,
and we don't need to have a conditional pfree. What I did not like about this
idea after thinking about it more is the performance overhead potentially since
every call to the view will take a shared lock on the entries, and the stats
will not be consistent when the view is called multiple times in a transaction
even when stats_fetch_consistency is NONE. The latter could be a desired
feature, but it goes against the users intentions and could be confusing.
I went ahead and implemented Andres's idea of will_free. Callers of
pgstat_fetch_entry can either pass a NULL to a will_free parameter,
or a bool. Callers that pass the bool can check if will_free is true and
can choose to free the entry.
For now, to keep the changes minimal, I only pgstat_fetch_stat_tabentry_ext()
will call pgstat_fetch_entry() with the bool and relation_needs_vacanalyze()
will be the only call site that checks this to pfree the entry.
There may be some opportunities to improve other call sites if they
are indeed leaking.
For example, pgstat_copy_relation_stats() could leak with
fetch_consistency = NONE. I kept
that out for now, but we should probably close that gap in another patch.
Also, pgstat_fetch_stat_dbentry() in autovacuum.c could potentially
use this, but I did
not look into detail.
What do you think?
--
Sami
