Feedback-ID: i0fe9450f:Fastmail
Date: Thu, 11 Jul 2024 13:58:19 +0900
From: Michael Paquier <michael@paquier.xyz>
To: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Cc: pgsql-hackers@lists.postgresql.org
Subject: Re: relfilenode statistics
Message-ID: <Zo9j69GhexDpeV4k@paquier.xyz>
References: <ZlGYokUIlERemvpB@ip-10-97-1-34.eu-west-3.compute.internal>
 <Zo4j-mC0Ps3vnjZ4@paquier.xyz>
 <Zo6OvsBLUbYm6nQM@ip-10-97-1-34.eu-west-3.compute.internal>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="1sqRfz4FfrHuBo9Z"
Content-Disposition: inline
In-Reply-To: <Zo6OvsBLUbYm6nQM@ip-10-97-1-34.eu-west-3.compute.internal>
Archived-At: <https://www.postgresql.org/message-id/Zo9j69GhexDpeV4k%40paquier.xyz>
Precedence: bulk


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

On Wed, Jul 10, 2024 at 01:38:06PM +0000, Bertrand Drouvot wrote:
> On Wed, Jul 10, 2024 at 03:02:34PM +0900, Michael Paquier wrote:
>> and I am troubled by the approach taken (mentioned down by you), but
>> that's invasive compared to how pgstats wants to be transparent with
>> its stats kinds.
>>=20
>> +   Oid         objoid;         /* object ID, either table or function
>> or tablespace. */
>> +   RelFileNumber relfile;      /* relfilenumber for RelFileLocator. */
>>  } PgStat_HashKey;
>>=20
>> This adds a relfilenode component to the central hash key used for the
>> dshash of pgstats, which is something most stats types don't care
>> about.
>=20
> That's right but that's an existing behavior without the patch as:
>=20
> PGSTAT_KIND_DATABASE does not care care about the objoid
> PGSTAT_KIND_REPLSLOT does not care care about the dboid
> PGSTAT_KIND_SUBSCRIPTION does not care care about the dboid
>=20
> That's 3 kinds out of the 5 non fixed stats kind.

I'd like to think that this is just going to increase across time.

>> That looks like the incorrect thing to do to me, particularly
>> seeing a couple of lines down that a stats kind is assigned so the
>> HashKey uniqueness is ensured by the KindInfo:
>> +   [PGSTAT_KIND_RELFILENODE] =3D {
>> +       .name =3D "relfilenode",
>=20
> You mean, just rely on kind, dboid and relfile to ensure uniqueness?

Or table OID for the objid, with a hardcoded number of past
relfilenodes stats stored, to limit bloating the dshash with too much
past stats.  See below.

> So, I think it makes sense to link the hashkey to all the RelFileLocator
> fields, means:
>=20
> dboid (linked to RelFileLocator's dbOid)
> objoid (linked to RelFileLocator's spcOid)
> relfile (linked to RelFileLocator's relNumber)

Hmm.  How about using the table OID as objoid, but store in the stats
of the new KindInfo an array of entries with the relfilenodes (current
and past, perhaps with more data than the relfilenode to ensure the
uniqueness tracking) and each of its stats?  The number of past
relfilenodes would be fixed, meaning that there would be a strict
control with the retention of the past stats.  When a table is
dropped, removing its relfilenode stats would be as cheap as when its
PGSTAT_KIND_RELATION is dropped.

> Yeah, I also thought about keeping a list of "previous" relfilenodes stat=
s for a
> relation but that would lead to:
>=20
> 1. Keep previous relfilnode stats=20
> 2. A more complicated way to look at relation stats (as you said)
> 3. Extra memory usage
>=20
> I think the only reason "previous" relfilenode stats are needed is to pro=
vide
> accurate stats for the relation. Outside of this need, I don't think we w=
ould
> want to retrieve "individual" previous relfilenode stats in the past.
>=20
> That's why the POC patch "simply" copies the stats to the relation during=
 a
> rewrite (before getting rid of the "previous" relfilenode stats).

Hmm.  Okay.
--
Michael

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

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

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmaPZmsACgkQnvQgOdby
QH3ZrA//bz5KcITcr5uq/tfIcPptLR3f1hnTWOoSZyCNRQq08FoWzclkESc2ucZS
xOBvaHXrBDjRLYvTkXlibZQBs+Uj+qTERlH2GqzWXF+ZL+PVuyOaTMe5lGsxT+nA
iszxJ1TtqXZ46us9Vszyr8CnM2GT8zxVwXzVuFxQin2YJEYhHlzdspe3OfAekt49
IHhgn9xC6/u9pWdOutnNlj93C4abAq9oN4hcum/loYM2aPzBSrCkxjnQg68h1/72
9qP56i58Tf9I66/Z1dsSIxckjR8eZM8BXcMGjKPiJbP6UZhpDTscAfeXHsCcwCD2
wcLOQJVTlunZ6oBmfwFbx4RZZoPkgd3a9Z/r1/4FlMQKvBWgpSE1x71UiQMmLxdb
prg2be0/7S4zZjO/+6nzCRtXFTh+9d6kCzSPzcby6xLhu2ceMaJM2Vx5K4siVbNb
iiNeR6zpbDi3jVIUlVmXFxZHj5WJbZ8gshYh1mqVn5kY8aVevoDS0Xm75dSFVt0X
5NCoxPmDJbv0i+R5PMaM8h0eg59Lds7UI3oI8CpIj32hTfZ6CFj1FsGWVx9FPbyY
KAeSm/mTYF91ar+I+VFJo4rJV+VW1sjH1zgEhcpcH04K7xsnXUkNRDeg6QPrCUSC
0Hggy+uG92pQmZR5+r89ed+kzhxwJE4fZfx4MoCjdxeli4z9vaM=
=Yxvg
-----END PGP SIGNATURE-----

--1sqRfz4FfrHuBo9Z--