Date: Sat, 10 Apr 2021 20:55:37 -0500
From: Justin Pryzby <pryzby@telsasoft.com>
To: Alvaro Herrera <alvherre@alvh.no-ip.org>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, pgsql-hackers@lists.postgresql.org,
	Zhihong Yu <zyu@yugabyte.com>
Subject: Re: pgsql: autovacuum: handle analyze for partitioned tables
Message-ID: <20210411015537.GA14564@telsasoft.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20210409155430.GA9512@alvherre.pgsql>
User-Agent: Mutt/1.9.4 (2018-02-28)
Archived-At: 
 <https://www.postgresql.org/message-id/20210411015537.GA14564%40telsasoft.com>
Precedence: bulk

On Thu, Apr 08, 2021 at 04:11:49PM -0400, Alvaro Herrera wrote:
> On 2021-Apr-08, Tom Lane wrote:
> 
> > > So I tend to think that my initial instinct was the better direction: we
> > > should not be doing any find_all_inheritors() here at all, but instead
> > > rely on pg_class.reltuples to be set for the partitioned table.
> > 
> > +1
> 
> This patch does that.

|commit 0e69f705cc1a3df273b38c9883fb5765991e04fe
|Author: Alvaro Herrera <alvherre@alvh.no-ip.org>
|Date:   Fri Apr 9 11:29:08 2021 -0400
|
|    Set pg_class.reltuples for partitioned tables
|    
|    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.

+       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);
+       }

This refers to "relpages", but I think it means "reltuples".

src/include/commands/vacuum.h:extern void vac_update_relstats(Relation relation,
src/include/commands/vacuum.h-                                BlockNumber num_pages,
src/include/commands/vacuum.h-                                double num_tuples,
src/include/commands/vacuum.h-                                BlockNumber num_all_visible_pages,

I'm adding it for the next round of "v14docs" patch if you don't want to make a
separate commit for that.

-- 
Justin