Hi, On Wed, Sep 04, 2024 at 03:12:37PM +0900, Michael Paquier wrote: > On Wed, Sep 04, 2024 at 05:28:56AM +0000, Bertrand Drouvot wrote: > > On Wed, Sep 04, 2024 at 02:05:46PM +0900, Kyotaro Horiguchi wrote: > >> The generalization sounds good to me, and hiding the private flags in > >> private places also seems good. > > > > +1 on the idea. > > Thanks for the feedback. > > >> Regarding pgstat_io_flush_cb, I think it no longer needs to check the > >> have_iostats variable, and that check should be moved to the new > >> pgstat_flush_io(). The same applies to pgstat_wal_flush_cb() (and > >> pgstat_flush_wal()). As for pgstat_slru_flush_cb, it simply doesn't > >> need the check anymore. > > > > Agree. Another option could be to add only one callback (the flush_fixed_cb > > one) > > and get rid of the have_fixed_pending_cb one. Then add an extra parameter > > to the > > flush_fixed_cb that would only check if there is pending stats (without > > flushing them). I think those 2 callbacks are highly related that's why I > > think we could "merge" them, thoughts? > > I would still value the shortcut that we can use if there is no > activity to avoid the clock check with GetCurrentTimestamp(),
Agree. The idea was to add an additional parameter (say "check_only") to the flush_fixed_cb. If this parameter is set to true then the flush_fixed_cb would do nothing (no flush at all) but return a boolean that would indicate if there is pending stats. In case it returns false, then we could avoid the clock check. Regards, -- Bertrand Drouvot PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
