Feedback-ID: i0fe9450f:Fastmail
Date: Tue, 2 Jun 2026 15:30:40 +0900
From: Michael Paquier <michael@paquier.xyz>
To: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Cc: samimseih@gmail.com, lukas@fittl.com, pgsql-hackers@postgresql.org
Subject: Re: Improve pg_stat_statements scalability
Message-ID: <ah54kGcap-VuWd7y@paquier.xyz>
References: 
 <CAP53PkyWD99bNwFVv2KibfL4bhm-D78QAnc9UqQQJdtJ3mQWPQ@mail.gmail.com>
 <CAA5RZ0sQ+gDn-J85j1FzOdL1YjVYRegpmQpDiah1=REWZSZj+Q@mail.gmail.com>
 <CAA5RZ0uoxiQ2_=xHGRnyc4WdM9aR0fzdMhBubnw97po==--yGQ@mail.gmail.com>
 <20260601.135858.1116584574478485492.horikyota.ntt@gmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="pcSJjTYQvTResfOD"
Content-Disposition: inline
In-Reply-To: <20260601.135858.1116584574478485492.horikyota.ntt@gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/ah54kGcap-VuWd7y%40paquier.xyz>
Precedence: bulk


--pcSJjTYQvTResfOD
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

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.
>=20
> 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.
>=20
> 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

--pcSJjTYQvTResfOD
Content-Type: application/pgp-signature; name=signature.asc

-----BEGIN PGP SIGNATURE-----

iQIzBAEBCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmoeeJAACgkQnvQgOdby
QH1e0xAAiaRd4o3fQ0bhjA1AYTw1im6CntZtPS6ECfOaN4teI5HnNaBsn6+4IO7k
qXAxnT5BzFh4y16RHPCT60SPKiqWE4Y0/FWgdd1SN3JJWpJE62SFFCAakbeqyUL5
QFpW8Sv3JkRYaub7XAvtFxcJ18oqYTkhO3P5wv4iYwG4LLN3Wnv4o5lf4hdZE4Ko
qNIiYsphmlZi5781R/kHJB2bROGf29C04IiWvaivnq8ZFe2CyVIXb68TcP1aTp+j
1TP6kC8T0XjoSKzmuNlakqxMwgDXIPB4MSk5g+5MlEswLzl/SSThHU3cFIdU7HZy
8UZgt2zYTCO0jseuTMlkYAc+30yTxjZ8qz9HfiC2Gnv3o54bVA29MHE/Zr2Pd0mL
vY4PbYwBDKEDfiOe3gha41oGQGGtKpHt02tgzYMI1IfjGpJ9YNG6zuT1tFsOpf9N
Ehhh/7EuVF/yaiQ+tcJNtVe/KFkMdKGIWEkYIzEb6m2QlymGzVJcsIzghzgxWGFb
83c+4kXE9CRSH6TJd3eZY9v2Z2SsmCOHxDsriocYUnsNHm3L5seeiZUuZrpSden1
4QN9wTl80BAuBM4pNHfM1o0LWRaByfz3q27TQ0B5pm5BMpQWjqHvH1a+thr97AWz
UyOqEvO29mf3JbjTXh0HdlpfmhmzyN/MLDI7J1YrvcoHPvhL5Lg=
=Un+0
-----END PGP SIGNATURE-----

--pcSJjTYQvTResfOD--