Re: Fix handling of injection_points regarding pending stats

Hi,

On Fri, Dec 27, 2024 at 09:23:12AM +0900, Michael Paquier wrote:
> Hi all,
>
> While doing more stuff related to pgstats and WAL, I have bumped on
> the fact that the module injection_points uses for its
> variable-numbered stats a pending flush callback but it does not
> handle its entries so as these would be handled locally first, with a
> final flush timed by pgstat_report_stat() that would itself call the
> pending flush callback. This is a less efficient practice, because it
> requires to touch the pgstats dshash more often than necessary.
>
> This is caused by the use of pgstat_get_entry_ref_locked() rather than
> pgstat_prep_pending_entry() which would make sure to time the flush of
> any pending entries appropriately depending on pgstat_report_stat().
> I'd like to fix the module so as the flush timing is controlled like
> all the other stats kinds, which is a better and more efficient
> practice to have anyway as this code should stand as a template for
> custom pgstats kinds.

Agree that it makes sense to rewrite this part as done in this patch (as it
reflects better how things "should" be done).

> While doing so, I have also noticed that injection_stats_flush_cb()
> missed a pgstat_unlock_entry() when entries get locked.

Right.

> All that is addressed in the patch attached. Feel free if you have
> any comments.

The patch is pretty straightforward and LGTM.

Regards,

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