Feedback-ID: i0fe9450f:Fastmail
Date: Thu, 7 Nov 2024 17:16:51 +0900
From: Michael Paquier <michael@paquier.xyz>
To: Karina Litskevich <litskevichkarina@gmail.com>
Cc: Postgres hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: pg_stat_statements: Avoid holding excessive lock
Message-ID: <Zyx3c_dUmiGEN1Wh@paquier.xyz>
References: <CACiT8ibhCmzbcOxM0v4pRLH3abk-95LPkt7_uC2JMP+miPjxsg@mail.gmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="5M+apD7tAdVWvGot"
Content-Disposition: inline
In-Reply-To: <CACiT8ibhCmzbcOxM0v4pRLH3abk-95LPkt7_uC2JMP+miPjxsg@mail.gmail.com>
Archived-At: <https://www.postgresql.org/message-id/Zyx3c_dUmiGEN1Wh%40paquier.xyz>
Precedence: bulk


--5M+apD7tAdVWvGot
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Tue, Nov 05, 2024 at 08:37:08PM +0300, Karina Litskevich wrote:
> I suggest eliminating holding the excessive lock. See the attached patch.
> This would also restore the consistency between the code and the comments
> about entry's mutex spinlock usage.

You are right.  minmax_stats_since and stats_since are only set when
an entry is allocated or reset, so this is not going to matter.

> +		/*
> +		 * There is no need to hold entry->mutex when reading stats_since and
> +		 * minmax_stats_since for (unlike counters) they are always written
> +		 * while holding pgss->lock exclusively. We are holding pgss->lock
> +		 * shared so there should be no race here.
> +		 */
>  		stats_since = entry->stats_since;
>  		minmax_stats_since = entry->minmax_stats_since;
> -		SpinLockRelease(&entry->mutex);

The comment could be simpler, say a "The spinlock is not required when
reading these two as they are always updated when holding pgss->lock
exclusively.".  Or something like that.
--
Michael

--5M+apD7tAdVWvGot
Content-Type: application/pgp-signature; name="signature.asc"

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

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmcsd3MACgkQnvQgOdby
QH1E3Q//dVtD256Iw9kKYLhV1wgx/M6t0XqP1UKEBQ1z13+SM2D1Qpmi/eky6oje
pEGcyGfRrz1ID8jNmhwdbrO2YsFdm/EWgjlO2LDfryB64gzDlFlrzAWmrq/nrTtC
cJLDu/qiUxOymotpFWlzw13BtgURM2s8pkBxozMD0PM9Av80D/TQTeUoH88yJmWr
9U2HQAhezvyV5kx34EVRthQ0E7LPZ8DfbeHC796J6/Q/GTiqphW3uCbItZdtCybs
AXgv+AvmR06qG7uwuUudsia4tNnzVlDhLYGWIsKu8qApWziQQAke6EwP2Q8uD5dR
cH179dqOqBLqOAAAHC4lmsJJa3jbPJCJgJd8P6pTgdJjtql1E3AiwBk7OT/VlG0I
6wFsiFDjR0j3GLEPS63IGsgpd7mjXN4rSe/NiXDPYIU0RubOL+O7pAjMzGZXTFfp
uJ30/ruDdK8g1fTV/xBgEE1j0sBZ/E5yhJkOU0HS3JVnkcC6ie95Al5qZkcWU3kD
ujbUXkUYqCKuSNoqCjnMTQ1sI+gJaHDFVhIgBYlBSJUf2p8ms+o9M/pm3srlFrpL
8UZ/XTh2hf6Hsb8Q5Hr9tghu6468IqvOnORQ9w/eMT1awHJq1ObV1p1SGkVT7Bcq
l5ND8KVb0/bX5NSFCP3jYDbT+T5REu85rNv9VSlgLFLErohGP2A=
=hRf7
-----END PGP SIGNATURE-----

--5M+apD7tAdVWvGot--