public inbox for [email protected]  

help / color / mirror / Atom feed

From: Kyotaro Horiguchi <[email protected]>
To: [email protected]
Cc: [email protected]
Subject: Re: Add callbacks for fixed-numbered stats flush in pgstats
Date: Wed, 04 Sep 2024 14:05:46 +0900 (JST)
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>

At Tue, 3 Sep 2024 13:48:59 +0900, Michael Paquier <[email protected]> wrote in 
> Hi all,
> 
> The last TODO item I had in my bucket about the generalization of
> pgstats is the option to a better control on the flush of the stats
> depending on the kind for fixed-numbered stats.  Currently, this is
> controlled by pgstat_report_stat(), that includes special handling for
> WAL, IO and SLRU stats, with two generic concepts:
> - Check if there are pending entries, allowing a fast-path exit.
> - Do the actual flush, with a recheck on pending entries.
> 
> SLRU and IO control that with one variable each, and WAL uses a
> routine for the same called pgstat_have_pending_wal().  Please find
> attached a patch to generalize the concept, with two new callbacks
> that can be used for fixed-numbered stats.  SLRU, IO and WAL are
> switched to use these (the two pgstat_flush_* routines have been kept
> on purpose).  This brings some clarity in the code, by making
> have_iostats and have_slrustats static in their respective files.  The
> two pgstat_flush_* wrappers do not need a boolean as return result.

The generalization sounds good to me, and hiding the private flags in
private places also seems good.

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.

> Running Postgres on scissors with a read-only workload that does not
> trigger stats, I was not able to see a difference in runtime, but that
> was on my own laptop, and I am planning to do more measurements on a
> bigger machine.

I don't think it matters, since the actual flushing occurs at
10-second intervals during busy times. We could change the check from
a callback to a variable, but that would just shift the function call
overhead to a more frequently called side.

> This is in the same line of thoughts as the recent thread about the
> backend init callback, generalizing more the whole facility:
> https://www.postgresql.org/message-id/[email protected]
> 
> Like the other one, I wanted to send that a few days ago, but well,
> life likes going its own ways sometimes.

reards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center

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]
  Subject: Re: Add callbacks for fixed-numbered stats flush in pgstats
  In-Reply-To: <[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