Feedback-ID: i0fe9450f:Fastmail
Date: Fri, 7 Mar 2025 14:42:13 +0900
From: Michael Paquier <michael@paquier.xyz>
To: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Cc: Andres Freund <andres@anarazel.de>, Xuneng Zhou <xunengzhou@gmail.com>,
	pgsql-hackers@lists.postgresql.org
Subject: Re: per backend WAL statistics
Message-ID: <Z8qHNelALLU6LiSy@paquier.xyz>
References: <Z8Egplq4gz1OMiet@paquier.xyz>
 <Z8GBMEcjsUAtwgHa@ip-10-97-1-34.eu-west-3.compute.internal>
 <Z8UKZyVSHUUQJHNb@paquier.xyz>
 <Z8VzqpBkj6ja5+EN@ip-10-97-1-34.eu-west-3.compute.internal>
 <Z8WIwBMintNmw73t@ip-10-97-1-34.eu-west-3.compute.internal>
 <CABPTF7XABZskNXORto-u=JmMTcw14X9NARpc7Vf8iGMVxkcYvg@mail.gmail.com>
 <Z8hLi1dGfPFc8AYy@ip-10-97-1-34.eu-west-3.compute.internal>
 <clef2a7wqnopfjw3jl7pbn25gko6m3fk2zctmtqjgyx6a7xw5j@sujsipmj4zif>
 <Z8iJUHn13NsSCYhj@ip-10-97-1-34.eu-west-3.compute.internal>
 <Z8l6EMM4ImVoWRkg@ip-10-97-1-34.eu-west-3.compute.internal>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha256;
	protocol="application/pgp-signature"; boundary="M7zdMeQ771itEvJ7"
Content-Disposition: inline
In-Reply-To: <Z8l6EMM4ImVoWRkg@ip-10-97-1-34.eu-west-3.compute.internal>
Archived-At: 
 <https://www.postgresql.org/message-id/Z8qHNelALLU6LiSy%40paquier.xyz>
Precedence: bulk


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

On Thu, Mar 06, 2025 at 10:33:52AM +0000, Bertrand Drouvot wrote:
> Indeed, there is no reason for pgstat_backend_have_pending_cb() to return=
 true if=20
> pgstat_tracks_backend_bktype() is not satisfied.
>=20
> So it deserves a dedicated patch to fix this already existing issue:
> 0001 attached.

 pgstat_backend_have_pending_cb(void)
 {
-    return (!pg_memory_is_all_zeros(&PendingBackendStats,
-                                    sizeof(struct PgStat_BackendPending)));
+    if (!pgstat_tracks_backend_bktype(MyBackendType))
+        return false;
+    else
+        return (!pg_memory_is_all_zeros(&PendingBackendStats,
+                                        sizeof(struct PgStat_BackendPendin=
g)));

So, if I understand your point correctly, it is not a problem on HEAD
because we are never going to update PendingBackendStats in the
checkpointer as pgstat_count_backend_io_op[_time]() blocks any attempt
to do so.  However, it becomes a problem with the WAL portion patch
because of the dependency to pgWalUsage which may be updated by the
checkpointer and the pending callback would happily report true in
this case.  It would also become a problem if we add in backend stats
a different portion that depends on something external.

An extra check based on pgstat_tracks_backend_bktype() makes sense in
pgstat_backend_have_pending_cb(), yes, forcing the hand of the stats
reports to not do stupid things in a process where we should not
report stats.  Good catch from the sanity check in
pgstat_report_stat(), even if this is only a problem once we begin to
use something else than PendingBackendStats for the pending stats.
This has also the merit of making pgstat_report_stat() do less work.
--
Michael

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

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

iQIzBAEBCAAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmfKhzUACgkQnvQgOdby
QH1fRQ/9GzjLRRufMCzRt4GltKk2Q4lC8fxsxsNHUPuqt6p4B7NRD3HWFlBiq83E
OqxS3WKbO1QpgX7jPT9VQsGE3ncGZQbjf5wweI7BvR7zDAqt8UEHBBL5de/3A0fP
htl5XoLseLXG/oj8MYc7H0QRBiKOngGRsPFDUD0ioZ3Ihj2PdBDnR8Buto9sIUdi
TQ9UCxGFmlqLmAMIyC8BQ90Aua3ryfaqjCV561AER6gpnT397ZfHCyd0RWw7qBtJ
VzQEFthgHtf2duBErbXcsALGukCwUFmF2Pal033icEsGFHNEvxMPmmHIRL+RlrID
e5A9BMOBKw9PdAfkauuz6dQJEz5VwoOoVTQH3cXm69+k+7m3rz2KuZopKq7/ppba
XTtXuun/a1CyD4EGFo72K82lZtY66/Wl3X2+KkxRuJXiktb+7IzRLE5Jtzd8ZEHJ
xjCMJv7apsJjVmeX8Nz/KrrKxlJMdM9z2t1ZXLd5mMnc1r7i1WAaxq/jbdulECS4
plQTqXS4mSs4C6hej8i4GMO2flY7g0Hm2SWQ6n/CBe7bOoomvBa0ceD37XHoFlkL
c9pD/riVwjw6N9paO847KINDCoUiutmzlWY43PaSrAwr9NO7Jtjs1eaWIfa//lP4
w2mx4I4cNdVbbwfOtfVBA0TqOJKUEsupg3vD9XJcxcJDD2G42gw=
=zvU+
-----END PGP SIGNATURE-----

--M7zdMeQ771itEvJ7--