From: Tomas Vondra <tomas.vondra@enterprisedb.com>
To: Alvaro Herrera <alvherre@alvh.no-ip.org>
To: yuzuko <yuzukohosoya@gmail.com>
Cc: David Steele <david@pgmasters.net>
Cc: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Cc: Justin Pryzby <pryzby@telsasoft.com>
Cc: Daniel Gustafsson <daniel@yesql.se>
Cc: Amit Langote <amitlangote09@gmail.com>
Cc: Masahiko Sawada <masahiko.sawada@2ndquadrant.com>
Cc: Laurenz Albe <laurenz.albe@cybertec.at>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Cc: Greg Stark <stark@mit.edu>
Subject: Re: Autovacuum on partitioned table (autoanalyze)
Date: Sun, 4 Apr 2021 21:08:51 +0200
Message-ID: <73dc10a0-cc6a-5d12-7f0f-e7410fa76d9c@enterprisedb.com> (raw)
In-Reply-To: <20210403194234.GA18917@alvherre.pgsql>
References: <20210403194234.GA18917@alvherre.pgsql>
On 4/3/21 9:42 PM, Alvaro Herrera wrote:
> Thanks for the quick rework. I like this design much better and I think
> this is pretty close to committable. Here's a rebased copy with some
> small cleanups (most notably, avoid calling pgstat_propagate_changes
> when the partition doesn't have a tabstat entry; also, free the lists
> that are allocated in a couple of places).
>
> I didn't actually verify that it works.
>
Yeah, this approach seems much simpler, I think. That being said, I
think there's a couple issues:
1) I still don't understand why inheritance and declarative partitioning
are treated differently. Seems unnecessary nad surprising, but maybe
there's a good reason?
2) pgstat_recv_tabstat
Should it really reset changes_since_analyze_reported in both branches?
AFAICS if the "found" branch does this
tabentry->changes_since_analyze_reported = 0;
that means we lose the counter any time tabstats are received, no?
That'd be wrong, because we'd propagate the changes repeatedly.
3) pgstat_recv_analyze
Shouldn't it propagate the counters before resetting them? I understand
that for the just-analyzed relation we can't do better, but why not to
propagate the counters to parents? (Not necessarily from this place in
the stat collector, maybe the analyze process should do that.)
4) pgstat_recv_reportedchanges
I think this needs to be more careful when updating the value - the
stats collector might have received other messages modifying those
counters (including e.g. PGSTAT_MTYPE_ANALYZE with a reset), so maybe we
can get into situation with
(changes_since_analyze_reported > changes_since_analyze)
if we just blindly increment the value. I'd bet would lead to funny
stuff. So maybe this should be careful to never exceed this?
regards
--
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: tomas.vondra@enterprisedb.com, alvherre@alvh.no-ip.org, yuzukohosoya@gmail.com, david@pgmasters.net, horikyota.ntt@gmail.com, pryzby@telsasoft.com, daniel@yesql.se, amitlangote09@gmail.com, masahiko.sawada@2ndquadrant.com, laurenz.albe@cybertec.at, pgsql-hackers@lists.postgresql.org, stark@mit.edu
Subject: Re: Autovacuum on partitioned table (autoanalyze)
In-Reply-To: <73dc10a0-cc6a-5d12-7f0f-e7410fa76d9c@enterprisedb.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox