Hi,

On Tue, Dec 16, 2025 at 10:39:15AM -0500, Andres Freund wrote:
> On 2025-12-16 16:33:17 +0900, Michael Paquier wrote:
> > 
> > FWIW, I am also still troubled by the part of the proposed patch set
> > where we are trying to hide the idea of a partitioned table has a
> > relfilenode set by using its relid instead in the key for the data.
> > This leads to a huge amount of complexity in the patch, mainly to
> > store data for autovacuum that we do not need at the end:
> > - autovacuum discards partitioned tables in do_autovacuum(), so the
> > stats related to partitioned tables that we need to select the
> > relations does not matter.
> 
> I feel like that's an implementation wart that we ought to fix. It's not
> infrequently a problem that we don't automatically analyze partitioned
> tables. Weren't there even a couple threads on that on the list in the last
> weeks?
> 
> 
> > - manual vacuums may include partitioned tables to extract its
> > partitions, vacuum_rel() at the end discarding them.  Well, stats
> > don't matter anyway.
> > 
> > We only need to attach three fields to let autovacuum know if a
> > relation needs to run or not: dead_tuples, ins_since_vacuum,
> > mod_since_analyze.
> 
> That may be true for autovacuum today, but I don't see any reason for
> live_tuples, tuples_inserted etc to be inaccurate after a failover.
> 
> > Most the fields of PgStat_StatTabEntry make sense
> > only for tables, few are required by indexes for pg_stat_all_indexes.
> > Some fields actually make sense because they refer to on-disk files,
> > mostly for pg_statio_all_tables (blocks_fetched, blocks_hit).
> > 
> > Hence, why don't we split PgStat_StatTabEntry into three things from
> > the start, even if it means to duplicate some of them?  Say:
> > - Table fields: includes [auto]vacuum/analyze data, block fields,
> > fields of pg_stat_all_tables.
> 
> What do you mean with "block fields"? pg_statio_all_tables? If so, what's the
> point of including them here, rather than in the relfilenode fields?
> 
> 
> > - Index fields: no need for the [auto]vacuum/analyze time and counts,
> > block fields, pg_stat_all_indexes fields.
> 
> I think we actually should populate the [auto]vac fields for indexes, right
> now it's impossible to figure out from stats whether indexes are frequently
> scanned as part of vacuum or not.
> 
> 
> > - Relfilenode fields: dead_tuples, ins_since_vacuum and
> > mod_since_analyze.  Does not apply to partitioned tables and indexes,
> > only applies to tables.  Provides a clean split, embrace the fact that
> > these are the only three fields we need to worry about during
> > recovery.
> 
> I think we really ought to populate not just these during recovery, but also
> at least n_tup_ins, n_tup_upd, n_tup_del, n_tup_hot_upd, n_live_tup.
> 
> I don't understand why we would want to only populate these three fields?
> 
> 
> I'm not against splitting the index fields off, but it seems pretty orthogonal
> to what we're discussing here.  If we were to split of index stats into a
> separate stat, why wouldn't we keep the statio fields in the relfilenode
> stats, since they're obviously intimately tied to that?

Andres, Michael, let me try to sum up my understanding of the current state
and see how we could now move forward. 

First of all, I understand that you both think that the patch outcome will be
useful to have. The current debate is about the design, the current status is:

- Andres raised specific technical/implementation concerns and I've proposed
solutions in [1]. It also looks like Andres supports the overall design 
approach. 
- Michael is not really ok with the current design approach.

That means, that with the current design in place, Michael would probably not
commit it (even after review(s)).

Given that I'm also in favor of the current proposed design, this raises the
questions:

- Andres, would you commit such a patch (after review iteration(s) of course)?
- Michael, if Andres is ok with the above, would you still offer your help for 
the
review part (even if the design is not what you "prefer"/"like")?

[1]: 
https://postgr.es/m/aUEyzoOJtrCLAEeT%40ip-10-97-1-34.eu-west-3.compute.internal

Regards,

-- 
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com


Reply via email to