public inbox for [email protected]
help / color / mirror / Atom feedFrom: Michael Paquier <[email protected]>
To: Kyotaro Horiguchi <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Subject: Re: Improve pg_stat_statements scalability
Date: Tue, 2 Jun 2026 15:30:40 +0900
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <CAP53PkyWD99bNwFVv2KibfL4bhm-D78QAnc9UqQQJdtJ3mQWPQ@mail.gmail.com>
<CAA5RZ0sQ+gDn-J85j1FzOdL1YjVYRegpmQpDiah1=REWZSZj+Q@mail.gmail.com>
<CAA5RZ0uoxiQ2_=xHGRnyc4WdM9aR0fzdMhBubnw97po==--yGQ@mail.gmail.com>
<[email protected]>
On Mon, Jun 01, 2026 at 01:58:58PM +0900, Kyotaro Horiguchi wrote:
> As far as I can see, the current implementation does not apply any
> throttling to calls of this API. In theory, a large number of backends
> could invoke it frequently and generate a high flush rate. For
> example, if 1000 backends were to call it once per second, that would
> result in 1000 shared-stats updates per second.
>
> Whether such a usage pattern would occur in practice is a separate
> question. However, given that existing pgstat code uses
> PGSTAT_MIN_INTERVAL to limit flush frequency, it seems to me that
> anytime stats should probably retain a similar restriction.
Hmm, OK. One cost in this decision is that it could randomly make the
tests randomly slower, which is one reason why this patch has been
added to this thread.
> Historically, flushing and freeing an entry were effectively the same
> decision because flushing only happened after transaction end. With
> anytime flushes, however, those become separate concerns. A callback
> may be able to flush everything that is appropriate for the current
> flush context, while the caller may still need to keep the entry
> around for later transaction-end processing.
Noted.
> That makes it hard to reason about checks such as
> pg_memory_is_all_zeros(&lstats->counts, ...). This check still appears
> to tell whether the counters themselves are zero, but the proposed
> change makes it look as if that is no longer enough to determine whether
> the entry is "empty", because entry lifetime is also folded into the
> callback result.
>
> I wonder whether it would be cleaner for the caller to make the lifetime
> decision, based on the kind of flush being performed, and for the
> callback to be told that flush context explicitly. For example, the
> callback could be passed whether this is an anytime flush or a
> transaction-boundary flush, flush the counters that are appropriate for
> that context, and then return whether any counters remain.
So you would suggest to extend the flush callback(s) with a context
value instead of having each flush callback do their own decisions
depending on if we are in a transaction block or not, right?
Another question that I would have for you is: do you think that we
should try to keep pgstat_report_stat() as the sole entry point for
the flush of the stats? Or should we try to keep the same approach as
what this patch is doing with a new routine that loops through the
flush callbacks? FWIW, I am kind of finding the approach of having a
single entry point rather than two as more appealing with the
long-term picture in mind. That's just a single take, where we could
just pgstat_report_stat() an extra argument based on the context where
it is called, lifting its current !IsTransactionOrTransactionBlock()
requirement.
As you are the original author of this area of this code, I'd be
really interested in your opinion here.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, 2-signature.asc)
download
view thread (12+ 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: Improve pg_stat_statements scalability
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