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
