Date: Mon, 16 Mar 2020 15:02:31 +0900
From: Michael Paquier <michael@paquier.xyz>
To: Justin Pryzby <pryzby@telsasoft.com>
Cc: Julien Rouhaud <rjuju123@gmail.com>, Sergei Kornilov <sk@zsrv.org>,
	Guillaume Lelarge <guillaume@lelarge.info>,
	pgsql-hackers@lists.postgresql.org
Subject: Re: Expose lock group leader pid in pg_stat_activity
Message-ID: <20200316060231.GD2331@paquier.xyz>
References: <20200116074912.GA98418@paquier.xyz>
 <CAOBaU_Z6WcT8FJfcCMU_Bp+PTzUpTEY9BfBQE4gbGKpE5wpfwQ@mail.gmail.com>
 <20200316042752.GE26184@telsasoft.com>
 <20200316054341.GG26184@telsasoft.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="Il7n/DHsA0sMLmDu"
Content-Disposition: inline
In-Reply-To: <20200316054341.GG26184@telsasoft.com>
Precedence: bulk


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

On Mon, Mar 16, 2020 at 12:43:41AM -0500, Justin Pryzby wrote:
> I think I see.  Julien's v3 patch did this:
> https://www.postgresql.org/message-id/attachment/106429/pgsa_leader_pid-v=
3.diff
> +				if (proc->lockGroupLeader)
> +					values[29] =3D Int32GetDatum(proc->lockGroupLeader->pid);
>=20
> ..which is racy because a proc with a leader might die and be replaced by
> another proc without a leader between 1 and 2.
>=20
> But the code since v4 does:
>=20
> +				leader =3D proc->lockGroupLeader;
> +				if (leader)
> +                                       values[29] =3D Int32GetDatum(lead=
er->pid);
>=20
> ..which is safe because PROCs are allocated in shared memory, so leader i=
s for
> sure a non-NULL pointer to a PROC.  leader->pid may be read inconsistentl=
y,
> which is what the comment says: "no extra lock is being held".

Yes, you have the correct answer here.  As shaped, the code relies on
the state of a PGPROC entry in shared memory.
--
Michael

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

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

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAl5vFncACgkQnvQgOdby
QH1XRA//aBqA2IsbllMw7JV37sZy/vyeoGaNWjwlUtmAyZwNn3HOSo0vTkRwTLQ1
/h9TaSm/yesEWSLT7P3V50t+JXn84nCj8VBDJniam2dZaIjulWde0wS9laAjpL6j
ZOjEaCre0TypkSAP3v2Y+fK95iGpZc1ABzqu10uwfrCvLlIOOyJQMUtr/wRIu5Wr
YGVoYj5Te8N9EcxLbALPVjD81ICKTkOX7J6lv0wjLCrHWAlFMVGmbcSJYQ+aYTgo
C225BRHRQ1ge6oNFf/zqnVfvOBK1HgwG9pziP61Q0v688a7q1wVokWJwiBH8WOnN
1lfPxygApfF7LHkjjcb5i/5mRzIYQD8qN+3tUdaeRxiNvz/1YQYTqUclbq6VJKYJ
4WjioqI2Ot7QyhJgUvWMJ35ZUFWHSNY/JERzOih7cDEVW03FFRUeDMOeifoxkgge
s3Uo8RRRH1PyomijYAMKwmjBBdbnhBquxktLWaV6uslFhcC8I1iqEUCq8krKAxeg
B2Zzah6K+Nl8VsdX2kkA6lV12Xa9UmahJCCp+06iZ3o64Ysm1HfzwHKxGZ3h5KHE
GlwkYnoiljn0NShF6VjOpTO8n12UpBpHhNbrCVB+fb5L7CUTQl8QwaPUCl/Din57
Oq177Ac1uQ6tDelmbx05Of5vSaEQqyKlFLw9RxqP7QkBJvMFAbI=
=WC4x
-----END PGP SIGNATURE-----

--Il7n/DHsA0sMLmDu--