Date: Thu, 8 Apr 2021 11:27:57 -0400
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
To: Tomas Vondra <tomas.vondra@enterprisedb.com>
Cc: yuzuko <yuzukohosoya@gmail.com>, David Steele <david@pgmasters.net>,
	Kyotaro Horiguchi <horikyota.ntt@gmail.com>,
	Justin Pryzby <pryzby@telsasoft.com>,
	Daniel Gustafsson <daniel@yesql.se>,
	Amit Langote <amitlangote09@gmail.com>,
	Masahiko Sawada <masahiko.sawada@2ndquadrant.com>,
	Laurenz Albe <laurenz.albe@cybertec.at>,
	PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>,
	Greg Stark <stark@mit.edu>
Subject: Re: Autovacuum on partitioned table (autoanalyze)
Message-ID: <20210408152757.GA2691@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <9ea106b4-22fc-fac5-2451-e876572284aa@enterprisedb.com>
User-Agent: Mutt/1.10.1 (2018-07-13)
Archived-At: 
 <https://www.postgresql.org/message-id/20210408152757.GA2691%40alvherre.pgsql>
Precedence: bulk

On 2021-Apr-08, Tomas Vondra wrote:

> On 4/8/21 5:22 AM, 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.
> 
> How would that value get updated, for the parent?

Same as for any other relation: ANALYZE would set it, after it's done
scanning the table.  We would to make sure that nothing resets it to
empty, though, and that it doesn't cause issues elsewhere.  (The patch I
sent contains the minimal change to make it work, but of course that's
missing having other pieces of code maintain it.)

> > (Also, a minor buglet: if we do ANALYZE (col1), then ANALYZE (col2) a
> > partition, then we repeatedly propagate the counts to the parent table,
> > so we would cause the parent to be analyzed more times than it should.
> > Sounds like we should not send the ancestor list when a column list is
> > given to manual analyze.  I haven't verified this, however.)
> 
> Are you sure? I haven't tried, but shouldn't this be prevented by only
> sending the delta between the current and last reported value?

I did try, and yes it behaves as you say.

-- 
Álvaro Herrera       Valdivia, Chile
Bob [Floyd] used to say that he was planning to get a Ph.D. by the "green
stamp method," namely by saving envelopes addressed to him as 'Dr. Floyd'.
After collecting 500 such letters, he mused, a university somewhere in
Arizona would probably grant him a degree.              (Don Knuth)