On 2021-Apr-07, Alvaro Herrera wrote:
> However, I just noticed there is a huge problem, which is that the new
> code in relation_needs_vacanalyze() is doing find_all_inheritors(), and
> we don't necessarily have a snapshot that lets us do that. While adding
> a snapshot acquisition at that spot is a very easy fix, I hesitate to
> fix it that way, because the whole idea there seems quite wasteful: we
> have to look up, open and lock every single partition, on every single
> autovacuum iteration through the database. That seems bad. I'm
> inclined to think that a better idea may be to store reltuples for the
> partitioned table in pg_class.reltuples, instead of having to add up the
> reltuples of each partition. I haven't checked if this is likely to
> break anything.
I forgot to comment on this aspect. First, I was obviously mistaken
about there not being an active snapshot. I mean, it's correct that
there isn't. The issue is that it's really a bug to require that there
is one; it just hasn't failed before because partially detached
partitions aren't very common. So I patched that as a bug in a
preliminary patch.
Next, the idea of storing the number of tuples in pg_class.reltuples is
a nice one, and I think we should consider it in the long run. However,
while it can be done as a quick job (shown in the attached, which AFAICT
works fine) there are side-effects -- for example, TRUNCATE doesn't
clear the value, which is surely wrong. I suspect that if I try to
handle it in this way, it would blow up in some corner case I forgot to
consider. So, I decided not to go that way, at least for now.
--
Álvaro Herrera Valdivia, Chile
commit 5ddb7c00e5f1d63eb251d334afce738919a772c0
Author: Alvaro Herrera <alvhe...@alvh.no-ip.org>
AuthorDate: Wed Apr 7 23:52:33 2021 -0400
CommitDate: Wed Apr 7 23:52:33 2021 -0400
set pg_class.reltuples for partitioned tables
diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c
index 0789117bb8..36e35722bc 100644
--- a/src/backend/commands/analyze.c
+++ b/src/backend/commands/analyze.c
@@ -656,6 +656,17 @@ do_analyze_rel(Relation onerel, VacuumParams *params,
in_outer_xact);
}
}
+ else if (onerel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
+ {
+ /*
+ * Partitioned tables don't have storage, so we don't set any of these
+ * value in their pg_class entries. However, reltuples is necessary
+ * in order for auto-analyze to work properly, so update that.
+ */
+ vac_update_relstats(onerel, 0, totalrows,
+ 0, false, InvalidTransactionId, InvalidMultiXactId,
+ in_outer_xact);
+ }
/*
* Now report ANALYZE to the stats collector. For regular tables, we do
diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index 5517836be6..48c1bf048f 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -3208,44 +3208,7 @@ relation_needs_vacanalyze(Oid relid,
*/
if (PointerIsValid(tabentry) && AutoVacuumingActive())
{
- if (classForm->relkind != RELKIND_PARTITIONED_TABLE)
- {
- reltuples = classForm->reltuples;
- }
- else
- {
- /*
- * If the relation is a partitioned table, we must add up
- * children's reltuples.
- */
- List *children;
- ListCell *lc;
-
- reltuples = 0;
-
- /* Find all members of inheritance set taking AccessShareLock */
- children = find_all_inheritors(relid, AccessShareLock, NULL);
-
- foreach(lc, children)
- {
- Oid childOID = lfirst_oid(lc);
- HeapTuple childtuple;
- Form_pg_class childclass;
-
- childtuple = SearchSysCache1(RELOID, ObjectIdGetDatum(childOID));
- childclass = (Form_pg_class) GETSTRUCT(childtuple);
-
- /* Skip a partitioned table and foreign partitions */
- if (RELKIND_HAS_STORAGE(childclass->relkind))
- {
- /* Sum up the child's reltuples for its parent table */
- reltuples += childclass->reltuples;
- }
- ReleaseSysCache(childtuple);
- }
-
- list_free(children);
- }
+ reltuples = classForm->reltuples;
vactuples = tabentry->n_dead_tuples;
instuples = tabentry->inserts_since_vacuum;
anltuples = tabentry->changes_since_analyze;
