Set pg_class.reltuples for partitioned tables
authorAlvaro Herrera <alvherre@alvh.no-ip.org>
Fri, 9 Apr 2021 15:29:08 +0000 (11:29 -0400)
committerAlvaro Herrera <alvherre@alvh.no-ip.org>
Fri, 9 Apr 2021 15:50:33 +0000 (11:50 -0400)
When commit 0827e8af70f4 added auto-analyze support for partitioned
tables, it included code to obtain reltuples for the partitioned table
as a number of catalog accesses to read pg_class.reltuples for each
partition.  That's not only very inefficient, but also problematic
because autovacuum doesn't hold any locks on any of those tables -- and
doesn't want to.  Replace that code with a read of pg_class.reltuples
for the partitioned table, and make sure ANALYZE and TRUNCATE properly
maintain that value.

I found no code that would be affected by the change of relpages from
zero to non-zero for partitioned tables, and no other code that should
be maintaining it, but if there is, hopefully it'll be an easy fix.

Per buildfarm.

Author: Álvaro Herrera <alvherre@alvh.no-ip.org>
Reviewed-by: Zhihong Yu <zyu@yugabyte.com>
Discussion: https://postgr.es/m/1823909.1617862590@sss.pgh.pa.us

src/backend/commands/analyze.c
src/backend/commands/tablecmds.c
src/backend/postmaster/autovacuum.c

index 182a133033248a2ef48aeff5f72598a3a19d8623..cffcd543029de115439bee186519abd1c526d0b2 100644 (file)
@@ -656,6 +656,18 @@ 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 fields in
+        * their pg_class entries except for relpages, which is necessary for
+        * auto-analyze to work properly.
+        */
+       vac_update_relstats(onerel, -1, totalrows,
+                           0, false, InvalidTransactionId,
+                           InvalidMultiXactId,
+                           in_outer_xact);
+   }
 
    /*
     * Now report ANALYZE to the stats collector.  For regular tables, we do
index 1f19629a9494bb721a847ac73741073e76511b2b..f780918a9516419e964fa3f71cc99333be864e82 100644 (file)
@@ -337,6 +337,7 @@ typedef struct ForeignTruncateInfo
 static void truncate_check_rel(Oid relid, Form_pg_class reltuple);
 static void truncate_check_perms(Oid relid, Form_pg_class reltuple);
 static void truncate_check_activity(Relation rel);
+static void truncate_update_partedrel_stats(List *parted_rels);
 static void RangeVarCallbackForTruncate(const RangeVar *relation,
                                        Oid relId, Oid oldRelId, void *arg);
 static List *MergeAttributes(List *schema, List *supers, char relpersistence,
@@ -1755,6 +1756,7 @@ ExecuteTruncateGuts(List *explicit_rels,
 {
    List       *rels;
    List       *seq_relids = NIL;
+   List       *parted_rels = NIL;
    HTAB       *ft_htab = NULL;
    EState     *estate;
    ResultRelInfo *resultRelInfos;
@@ -1908,9 +1910,15 @@ ExecuteTruncateGuts(List *explicit_rels,
        Relation    rel = (Relation) lfirst(lc1);
        int         extra = lfirst_int(lc2);
 
-       /* Skip partitioned tables as there is nothing to do */
+       /*
+        * Save OID of partitioned tables for later; nothing else to do for
+        * them here.
+        */
        if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
+       {
+           parted_rels = lappend_oid(parted_rels, RelationGetRelid(rel));
            continue;
+       }
 
        /*
         * Build the lists of foreign tables belonging to each foreign server
@@ -2061,6 +2069,9 @@ ExecuteTruncateGuts(List *explicit_rels,
        ResetSequence(seq_relid);
    }
 
+   /* Reset partitioned tables' pg_class.reltuples */
+   truncate_update_partedrel_stats(parted_rels);
+
    /*
     * Write a WAL record to allow this set of actions to be logically
     * decoded.
@@ -2207,6 +2218,40 @@ truncate_check_activity(Relation rel)
    CheckTableNotInUse(rel, "TRUNCATE");
 }
 
+/*
+ * Update pg_class.reltuples for all the given partitioned tables to 0.
+ */
+static void
+truncate_update_partedrel_stats(List *parted_rels)
+{
+   Relation    pg_class;
+   ListCell   *lc;
+
+   pg_class = table_open(RelationRelationId, RowExclusiveLock);
+
+   foreach(lc, parted_rels)
+   {
+       Oid         relid = lfirst_oid(lc);
+       HeapTuple   tuple;
+       Form_pg_class rd_rel;
+
+       tuple = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(relid));
+       if (!HeapTupleIsValid(tuple))
+           elog(ERROR, "could not find tuple for relation %u", relid);
+       rd_rel = (Form_pg_class) GETSTRUCT(tuple);
+       if (rd_rel->reltuples != (float4) 0)
+       {
+           rd_rel->reltuples = (float4) 0;
+
+           heap_inplace_update(pg_class, tuple);
+       }
+
+       heap_freetuple(tuple);
+   }
+
+   table_close(pg_class, RowExclusiveLock);
+}
+
 /*
  * storage_name
  *   returns the name corresponding to a typstorage/attstorage enum value
index aef9ac4dd22ee70cdfee42785279b978e62bfaff..a799544738e5b2c25c06de7893700cc6491d44a3 100644 (file)
@@ -3209,44 +3209,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;