Re: Add support for entry counting in pgstats

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Sami Imseih <samimseih(at)gmail(dot)com>
Cc: Postgres hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Add support for entry counting in pgstats
Date: 2025-09-26 03:56:12
Message-ID: aNYO3AndLHHqEv16@paquier.xyz
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Sep 25, 2025 at 07:47:48PM -0500, Sami Imseih wrote:
> I spent a bit of time testing this with a pg_stat_statements like extension
> using a custom stats kind, and while I think there is value for both "live"
> ( ! ->dropped) counter and an exact dshash counter ( current proposal),
> I rather go with the latter at least initially, for the sake of not having 2
> atomic counters. Both will allow an extension to trigger some type of a
> cleanup strategy, and in general, both should be very close in value.
> That's at least my observation.

Thanks for the feedback. I'm feeling encouraged by this opinion about
the "hard" counter, so I'd like to move on with it.

> I do think however that the placement of the decrement is wrong, and that
> it should go inside pgstat_free_entry, since pgstat_free_entry can be called
> in multiple code paths. In my high churn workload, v2 ended up increasing way
> beyond the actual size of the dshash. See the attached for what
> I did to fix.

Yes, you are right and the patch is wrong. I've done the following
with my previous patch with injection_points loaded and its stats
activated, confirming your argument:
1) session with psql running in a tight loop and these queries:
select injection_points_attach('popo', 'notice');
select injection_points_run('popo');
select injection_points_detach('popo');
2) session that fetches the stats entry
select injection_points_stats_numcalls('popo');
\watch 0
3) session that checks the number of entries, should be 0 or 1:
select injection_points_stats_count();
\watch 1

And the numbers reported by the third session increase over time,
which is no good, once my patch is used. Once we move the
decrementation to pgstat_free_entry() as you propose, the counter is
under control and capped.

> IMO, "entry_counts" does not sound correct. We are not tracking more
> than one count. What about track_num_entries ?

Hmm. track_entry_counts resonates with the field name entry_counts,
and it's true that using the plural form is confusing. How about
track_entry_count in PgStat_KindInfo instead?
--
Michael

Attachment Content-Type Size
v3-0001-Add-support-for-entry-counting-in-pgstats.patch text/x-diff 6.8 KB
v3-0002-injection_points-Add-entry-counting.patch text/x-diff 4.6 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Bertrand Drouvot 2025-09-26 05:26:08 Re: Add memory_limit_hits to pg_stat_replication_slots
Previous Message Rishu Bagga 2025-09-26 03:36:07 Re: [PATCH] Write Notifications Through WAL