Horton

Thread

Commits

  1. injection_points: Tweak variable-numbered stats to work with pending data

  1. Fix handling of injection_points regarding pending stats

    Michael Paquier <michael@paquier.xyz> — 2024-12-27T00:23:12Z

    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.

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

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

Thanks,
--
Michael

  2. Re: Fix handling of injection_points regarding pending stats

    Bertrand Drouvot <bertranddrouvot.pg@gmail.com> — 2024-12-30T08:34:54Z

    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

  3. Re: Fix handling of injection_points regarding pending stats

    Michael Paquier <michael@paquier.xyz> — 2024-12-30T10:00:39Z

    On Mon, Dec 30, 2024 at 08:34:54AM +0000, Bertrand Drouvot wrote:
> On Fri, Dec 27, 2024 at 09:23:12AM +0900, Michael Paquier wrote:
>> All that is addressed in the patch attached.  Feel free if you have
>> any comments.
> 
> The patch is pretty straightforward and LGTM.

Thanks for double-checking.
--
Michael