Re: [PATCH] pgstat: Add helper to drop entries from a stats kind's hash

From: Bertrand Drouvot <bertranddrouvot(dot)pg(at)gmail(dot)com>
To: Sami Imseih <samimseih(dot)pg(at)gmail(dot)com>
Cc: Postgres hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: [PATCH] pgstat: Add helper to drop entries from a stats kind's hash
Date: 2026-10-28 13:03:09
Message-ID: auHyja6zbCQxtmA4@bdtpg
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

On Tue, Oct 27, 2026 at 04:18:36PM -0500, Sami Imseih wrote:
> Hi,
>
> 44539786b82 allowed a variable-numbered stats kind to use its own
> dshash. One remaining gap is that pgstat_drop_matching_entries() still
> scans all variable-numbered stats hash tables, even when the caller
> already knows which stats kind it wants.

Yeah, the approach makes sense to me.

> Because pgStatLocal.kind_hash[kind] may be the shared variable stats
> hash, the helper filters by kind before using the match callback. A nice
> side effect is that callers of the new function do not need to repeat the kind
> check in their match callbacks.

Agreed. I think filtering before the callback is actually an important behavior.

A couple of comments:

>
> The test_custom_stats module is adjusted so its existing named drop path
> uses the new function with a small match callback.

=== 1

+ if (filter_kind != PGSTAT_KIND_INVALID &&
+ ps->key.kind != filter_kind)
+ continue;

It looks like the test does not ensure this filtering happens before calling
do_drop(). Indeed, removing the above block still makes all 32 subtests pass,
as the test callback only compares objid.

I wonder if test_custom_stats_var_match_entry() could report an error when it
receives an unexpected kind? That would make this test deterministic, provided
another kind has an entry in the shared hash.

=== 2

- /* Drop entry and request GC if the entry could not be freed */
- if (!pgstat_drop_entry(PGSTAT_KIND_TEST_CUSTOM_VAR_STATS, InvalidOid,
- PGSTAT_CUSTOM_VAR_STATS_IDX(stat_name), false))
- pgstat_request_entry_refs_gc();
+ pgstat_drop_matching_entries_of_kind(PGSTAT_KIND_TEST_CUSTOM_VAR_STATS,
+ test_custom_stats_var_match_entry,
+ UInt64GetDatum(stat_idx));

AFAICS, this removes the only coverage of pgstat_drop_entry() with an own_hash
kind.

I wonder if it wouldn't make sense to keep the existing path for one entry and
use the new helper for the other one?

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Greg Burd 2026-10-28 13:29:39 Re: WAL record CRC calculated incorrectly because of underlying buffer modification
Previous Message Bertrand Drouvot 2026-10-28 12:34:24 Re: Report buffer access strategies in EXPLAIN