public inbox for [email protected]  

help / color / mirror / Atom feed

From: Bertrand Drouvot <[email protected]>
To: Michael Paquier <[email protected]>
Cc: Kyotaro Horiguchi <[email protected]>
Cc: [email protected]
Subject: Re: Add callbacks for fixed-numbered stats flush in pgstats
Date: Wed, 4 Sep 2024 06:27:43 +0000
Message-ID: <Ztf938t7Wt4//[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>

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

view thread (7+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Add callbacks for fixed-numbered stats flush in pgstats
  In-Reply-To: <Ztf938t7Wt4//[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox