Feedback-ID: i0fe9450f:Fastmail
Date: Mon, 25 Sep 2023 12:47:41 +0900
From: Michael Paquier <michael@paquier.xyz>
To: Ryoga Yoshida <bt23yoshidar@oss.nttdata.com>
Cc: pgsql-hackers@lists.postgresql.org
Subject: Re: Doesn't pgstat_report_wal() handle the argument "force"
 incorrectly
Message-ID: <ZREC3d5Aya0KPbSl@paquier.xyz>
References: <f87a4d7be70530606b864fd1df91718c@oss.nttdata.com>
 <ZRDatnXJqlx46zV9@paquier.xyz>
 <2f6f68343fcd244ef7be5dfae8e55f63@oss.nttdata.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="PA7b0eEg0wqGviRY"
Content-Disposition: inline
In-Reply-To: <2f6f68343fcd244ef7be5dfae8e55f63@oss.nttdata.com>
Archived-At: <https://www.postgresql.org/message-id/ZREC3d5Aya0KPbSl%40paquier.xyz>
Precedence: bulk


--PA7b0eEg0wqGviRY
Content-Type: multipart/mixed; boundary="l2HzZ57UGNURYJy4"
Content-Disposition: inline


--l2HzZ57UGNURYJy4
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Mon, Sep 25, 2023 at 11:27:27AM +0900, Ryoga Yoshida wrote:
> Thank you for the review. Certainly, adding a comments is a good idea. I
> added a comment.

Hmm.  How about the attached version with some tweaks?
--
Michael

--l2HzZ57UGNURYJy4
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v2-0002-bug-fix-in-foce-within-pgstat_report_wal.patch"
Content-Transfer-Encoding: quoted-printable

diff --git a/src/backend/utils/activity/pgstat_wal.c b/src/backend/utils/ac=
tivity/pgstat_wal.c
index bcaed14d02..82feb792cf 100644
--- a/src/backend/utils/activity/pgstat_wal.c
+++ b/src/backend/utils/activity/pgstat_wal.c
@@ -38,13 +38,18 @@ static WalUsage prevWalUsage;
  *
  * Must be called by processes that generate WAL, that do not call
  * pgstat_report_stat(), like walwriter.
+ *
+ * force set to true ensures that the statistics are flushed; note that
+ * this needs to acquire the pgstat shmem LWLock, waiting on it.  When
+ * set to false, the statistics may not be flushed if the lock could not
+ * be acquired.
  */
 void
 pgstat_report_wal(bool force)
 {
-	pgstat_flush_wal(force);
-
-	pgstat_flush_io(force);
+	/* like in pgstat.c, don't wait for lock acquisition when !force */
+	pgstat_flush_wal(!force);
+	pgstat_flush_io(!force);
 }
=20
 /*

--l2HzZ57UGNURYJy4--

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

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

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmURAt0ACgkQnvQgOdby
QH00nxAAnL8zbEwf67IiAtmYI/IiHTHMumtlIU1ARhiE0v+Gmr27YgcJPuduDQ4l
QmpIUab+bodoturl2WadNu+lbCMBHZG0m3zGO0/E9kFzQ5585m14JmueI2Px5/ZS
TzGVihVU06U41UPp93zHHtU2uCQi8mKIAaT678sApan86hTao9+hgH0xMZ/AAPd2
tB4KVtI3PL76v47Yu+pms4z12yNvoBP8yHwmaOg3oVKw/ujMF4OcuqJ8CfQ2KDBY
rcq4NEwKUeIgh5KwQbCKaQMtekqhsOZ1OUuv56No7wMC5N6KDLOelJmyeAhraEcc
G5BoxCHpEf++Obj2JwWgkSzjPG1UTo1gVdG0AjFHeH6VneEB6S1F/+ItMAGq+Y00
gUUIWc2gjd4UVf4AktH6TQX9UFDLAK1Gs1+MgndkDwtzRckke6oTjsxLiwmZjlcx
0/YwRDvQNDfZi3UNUX4CAOJqs3uvDqG11w+B+XKosMKWCdmWX4dXcETCzlI6fICv
5aJeJjZ48IP1CBPoXwSsA39YQNyGN3hc81/7umrlewDfd2BYK2DppMAIrhF5qVSP
GUKEW26IMrLRZPr7E2vXz9tCwDv4iHpZEBNaho6FNHtB7RM3igdMwgllRrQTCLsC
c6/RV8nB0Jdi2Anyb6Fd7xLQ7Cnw4YK32QaxJYw74m9d+ai3Ze8=
=g7/6
-----END PGP SIGNATURE-----

--PA7b0eEg0wqGviRY--