public inbox for [email protected]
help / color / mirror / Atom feedFrom: Bertrand Drouvot <[email protected]>
To: Andres Freund <[email protected]>
Cc: Michael Paquier <[email protected]>
Cc: Kirill Reshke <[email protected]>
Cc: Robert Haas <[email protected]>
Cc: [email protected]
Subject: Re: relfilenode statistics
Date: Tue, 16 Dec 2025 10:22:06 +0000
Message-ID: <[email protected]> (raw)
In-Reply-To: <ezbfcpjtfh3vtm667aegyrptcdsvinbuxw6y5p6j3e5fbffrl5@i7v6cjapz3yv>
References: <[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<[email protected]>
<ezbfcpjtfh3vtm667aegyrptcdsvinbuxw6y5p6j3e5fbffrl5@i7v6cjapz3yv>
Hi,
On Mon, Dec 15, 2025 at 12:48:25PM -0500, Andres Freund wrote:
> On 2025-12-15 16:29:18 +0000, Bertrand Drouvot wrote:
> > From 7908ba56cb8b6255b869af6be13077aa0315d5f1 Mon Sep 17 00:00:00 2001
>
> I think this needs to make more explicit that this works because the object ID
> now is a uint64, and that both the inputs are 32 bits.
Yeah, it's now added in the commit message (mentioning b14e9ce7d55c).
> > diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
> > index 62035b7f9c3..a9b2b4e1033 100644
> > --- a/src/backend/access/heap/vacuumlazy.c
> > +++ b/src/backend/access/heap/vacuumlazy.c
> > @@ -961,8 +961,7 @@ heap_vacuum_rel(Relation rel, const VacuumParams params,
> > * soon in cases where the failsafe prevented significant amounts of heap
> > * vacuuming.
> > */
> > - pgstat_report_vacuum(RelationGetRelid(rel),
> > - rel->rd_rel->relisshared,
> > + pgstat_report_vacuum(rel->rd_locator,
> > Max(vacrel->new_live_tuples, 0),
> > vacrel->recently_dead_tuples +
> > vacrel->missed_dead_tuples,
>
> Why not pass in the Relation itself? Given that we do that already for
> pgstat_report_analyze(), it seems like that'd be an improvement even
> independent of this change?
Makes sense, done in [1].
> > diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
> > index 1bd3924e35e..563a3697690 100644
> > --- a/src/backend/postmaster/autovacuum.c
> > +++ b/src/backend/postmaster/autovacuum.c
> > @@ -2048,8 +2048,7 @@ do_autovacuum(void)
> >
> > /* Fetch reloptions and the pgstat entry for this table */
> > relopts = extract_autovac_opts(tuple, pg_class_desc);
> > - tabentry = pgstat_fetch_stat_tabentry_ext(classForm->relisshared,
> > - relid);
> > + tabentry = pgstat_fetch_stat_tabentry_ext(relid);
> >
> > /* Check if it needs vacuum or analyze */
> > relation_needs_vacanalyze(relid, relopts, classForm, tabentry,
>
> I don't think this is good - now do_autovacuum() will do a separate syscache
> lookup to fetch information the caller already has (due to the
> pgstat_reloid_to_relfilelocator() in pgstat_fetch_stat_tabentry_ext()). That's
> not too bad for things like viewing stats, but do_autovacuum() does this for
> every table in a database...
Good point. In the attached I added pgstat_fetch_stat_tabentry_by_locator().
It's called directly in do_autovacuum() and also in pgstat_fetch_stat_tabentry_ext().
I did not check if there are other places where we can call pgstat_fetch_stat_tabentry_by_locator()
directly. I want first to validate this idea makes sense, does it?
> I don't think this is true as stated. Two reasons:
>
> 1) This afaict guarantees that the relfilenode will not class with oids, but
> it does *NOT* guarantee that it does not clash with other relfilenodes
> 2) Note that GetNewRelFileNumber() does *NOT* check for conflicts when
> creating a new relfilenode for an existing relation:
> * If the relfilenumber will also be used as the relation's OID, pass the
> * opened pg_class catalog, and this routine will guarantee that the result
> * is also an unused OID within pg_class. If the result is to be used only
> * as a relfilenumber for an existing relation, pass NULL for pg_class.
Oh right, in case of OID wraparound. In the attached I added a new
"
#define PSEUDO_PARTITION_TABLE_SPCOID 1665
"
to ensure uniqueness then.
[1]: https://www.postgresql.org/message-id/flat/aUEA6UZZkDCQFgSA%40ip-10-97-1-34.eu-west-3.compute.intern...
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: relfilenode statistics
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox