Re: Improve pg_stat_statements scalability

Hello,

I'm a bit concerned that this exposes some of the underlying dshash
implementation details to stats kinds. I'd prefer to keep those
details encapsulated if possible. That said, considering the
performance implications, I can understand the direction taken by this
series.

0003:
> +bool
> +pgstat_drop_current(PgStatShared_HashEntry *shent, dshash_seq_status *hstat)

However, pgstat_drop_current() seems too tightly coupled to the state
of the underlying dshash scan. shent must be the current entry of
exactly the scan represented by hstat, but that relationship is not
expressed by the API.

As far as I can see, the function only combines two operations:
releasing a local entry reference, if any, and calling
pgstat_drop_entry_internal() for the current scan entry. Would it be
better to keep these as separate operations, perhaps by adding a small
helper for releasing the local entry reference, rather than combining
them into pgstat_drop_current()?

> +bool
> +pgstat_has_pending(PgStatShared_HashEntry *shent)

I also wonder if this should be added at this point.

The name is rather vague, and the function is not used anywhere in
this series. Unless there is an immediate caller, I'd be inclined to
leave it out for now and add it later if a concrete need arises.

Regards,

--
Kyotaro Horiguchi
NTT Open Source Software Center