Feedback-ID: i0fe9450f:Fastmail
Date: Fri, 7 Feb 2025 15:07:18 +0900
From: Michael Paquier <michael@paquier.xyz>
To: Jeff Davis <pgsql@j-davis.com>
Cc: Corey Huinker <corey.huinker@gmail.com>,
	jian he <jian.universality@gmail.com>,
	Nathan Bossart <nathandbossart@gmail.com>,
	Bruce Momjian <bruce@momjian.us>,
	Matthias van de Meent <boekewurm+postgres@gmail.com>,
	Tom Lane <tgl@sss.pgh.pa.us>, Magnus Hagander <magnus@hagander.net>,
	Stephen Frost <sfrost@snowman.net>,
	Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>,
	Peter Smith <smithpb2250@gmail.com>,
	PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>,
	alvherre@alvh.no-ip.org
Subject: Re: Statistics Import and Export
Message-ID: <Z6WjFqX4f3LVMydD@paquier.xyz>
References: <CADkLM=fe4f=YVGHbOGiXV+W2SoJZtcNw1Ta8gYh5UUhxndaVgw@mail.gmail.com>
 <CACJufxGuRPfd70NwogkRE26cZcKGrR9gjG4OiH18tu=BnyezHA@mail.gmail.com>
 <CADkLM=fMuVwcxr2aocz4H=LVf2PZsvTNenqEod2PoY1OyCU+kw@mail.gmail.com>
 <bd72b9c9ce285350c5ce247f43b21bea39f24326.camel@j-davis.com>
 <CADkLM=cQi+sYpLY6BqybZ8ZWffSU4+1-aEQsShSGbrh3M=Dtcg@mail.gmail.com>
 <b40b81d38c3a87fdef61e4f7abfbc7f27c7fbcd9.camel@j-davis.com>
 <CACJufxG6K4EAUROhdr0wkzMh5QyFLmdLZeAoh7Vh0-VbuAtHcw@mail.gmail.com>
 <CADkLM=fihosVw7fJak6Jq1AdCX8fuZ+7s1_-+AUZR=sdD13zAQ@mail.gmail.com>
 <CADkLM=cnYxaNFbAQRMLeOOga0j5pY_V6Tfcn+vjm_rXL=WOF7Q@mail.gmail.com>
 <0b4c098ae3968aa23d6accb691f47bdba143aedd.camel@j-davis.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="XDFYX0w6a4LLHgjt"
Content-Disposition: inline
In-Reply-To: <0b4c098ae3968aa23d6accb691f47bdba143aedd.camel@j-davis.com>
Archived-At: <https://www.postgresql.org/message-id/Z6WjFqX4f3LVMydD%40paquier.xyz>
Precedence: bulk


--XDFYX0w6a4LLHgjt
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Wed, Feb 05, 2025 at 08:45:06PM -0800, Jeff Davis wrote:
> v45-0001 addresses this by locking both the partitioned index, as well
> as its table, in ShareUpdateExclusive mode. That satisfies the in-place
> update requirement to take a ShareUpdateExclusiveLock on the
> partitioned index, while otherwise being the same as normal indexes
> (and therefore unlikely to cause a problem if ANALYZE sets stats on
> partitioned indexes in the future).
>=20
> That means:
>   *=A0For indexes: ShareUpdateExclusiveLock on table and AccessShareLock
> on index
>   * For partitioned indexes: ShareUpdateExclusiveLock on table and
> ShareUpdateExclusiveLock on index
>   * Otherwise, ShareupdateExclusiveLock on the relation
>=20
> which makes sense to me. The v45-0001 patch itself could use some
> cleanup, but I can take care of that at commit time if we agree on the
> locking scheme.

Fine by me.

The regression tests of v45 and v46 are a bit fuzzy regarding the
tests around locking for partitioned tables.  For example, with v46
applied on top of HEAD, if I manipulate the internals of the patch in
stats_check_arg_pair() so as we don't take a lock on the parent table,
then a make check is still happy and passes even if the internals are
clearly broken.

I would recommend to work a bit more the tests by updating the stats
of a relation with the SQL functions in a transaction and add some
queries on pg_locks for locktype =3D 'relation' that are able to check
the locks we are taking when running these operations (return pairs of
relation::regclass and mode, for example).  Doing that for
non-partitioned relations is also something I would do, so as the
locking schema we are using is clearly tracked and that future
manipulations of the area would help one in tracking problems.  Bonus
points: scans of pg_locks are cheap tests.
--
Michael

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

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

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmeloxYACgkQnvQgOdby
QH2elBAAgtVDlNGaHM5VoigVPaRAwnE77nJk5J/pzF2i3dqR/r6c2Zxr//7BRhs0
Zf+hStpeuHJ+5R+pbibczy8VfBkz0ZAVgkGtd8w/mB4lCGiIeW/FitSaxHMXEZuZ
myNWVcEjR6dT9a4epIhHnJqUCORTnbe7NThOmS+RPpdJrnAM2bRpSB1lLrthJrNy
oH9EplIXk84/7AGi1eE9JD/GcHGkoQQ2smwdF/I+OeKXqePuO3Cri3JCEjg2rLNB
u6hnGdtwUl8fFSO7cmCtu88h7qzlOFAUFU1S8rdKIj4KxjkP04Z0vLpLaHs8t9PC
xyCWuxPp7a2tr3eVnEWpz8/gC7GOarw5jtEp4wQqiq2hGaWG0Mxy4Uq+yF9+2Eus
1MsgIJbIcg6YGwQgFJi2haoVF4vzjM9roTum+rCfhBSHoLh6s6VPhWIu9JTncklG
nuByVia0oSSzb8ePIqsdshEcW1qORdwa2krplB/oGpGhGcgJwvsLTa98UgtENkJS
9gTs4ASNN1T0n6vjgmtJig2jaVzVyeqkn3R6nUV8IZWr78P2qNHIe2QTaAOBJAJW
6aAYssVZ74KxfyeUxmhjrUGWf6HfWFH1DgRlgC5sPTsXWOZniqugk7wNYVXS8sTL
YiU5UnHcHeb3L2c6axP/wPsBG4vvMUbLAI/7SLCNuBQ+mFoqVY0=
=D61V
-----END PGP SIGNATURE-----

--XDFYX0w6a4LLHgjt--