Re: Autovacuum on partitioned table

Hosoya-san,

On Thu, Feb 20, 2020 at 3:34 PM yuzuko <yuzukohosoya(at)gmail(dot)com> wrote:
> Attach the latest patch based on discussion in this thread.
>
> > > Yeah that is what I meant. In addition, adding partition's
> > > changes_since_analyze to its parent needs to be done recursively as
> > > the parent table could also be a partitioned table.
> >
> > That's a good point. So, changes_since_analyze increments are
> > essentially propagated from leaf partitions to all the way up to the
> > root table, including any intermediate partitioned tables. We'll need
> > to consider whether we should propagate only one level at a time (from
> > bottom of the tree) or update all parents up to the root, every time a
> > leaf partition is analyzed.
>
> For multi-level partitioning, all parents' changes_since_analyze will be
> updated whenever analyzing a leaf partition in this patch.
> Could you please check the patch again?

Thank you for the new patch.

I built and confirmed that the patch works.

Here are some comments:

* White-space noise in the diff (space used where tab is expected);
please check with git diff --check and fix.

* Names changes_tuples, m_changes_tuples should be changed_tuples and
m_changed_tuples, respectively?

* Did you intend to make it so that we now report *all* inherited
stats to the stats collector, not just those for partitioned tables?
IOW, do did you intend the new feature to also cover traditional
inheritance parents? I am talking about the following diff:

/*
- * Report ANALYZE to the stats collector, too. However, if doing
- * inherited stats we shouldn't report, because the stats collector only
- * tracks per-table stats. Reset the changes_since_analyze counter only
- * if we analyzed all columns; otherwise, there is still work for
- * auto-analyze to do.
+ * Report ANALYZE to the stats collector, too. If the table is a
+ * partition, report changes_since_analyze of its parent because
+ * autovacuum process for partitioned tables needs it. Reset the
+ * changes_since_analyze counter only if we analyzed all columns;
+ * otherwise, there is still work for auto-analyze to do.
*/
- if (!inh)
- pgstat_report_analyze(onerel, totalrows, totaldeadrows,
- (va_cols == NIL));
+ pgstat_report_analyze(onerel, totalrows, totaldeadrows,
+ (va_cols == NIL));

* I may be missing something, but why doesn't do_autovacuum() fetch a
partitioned table's entry from pgstat instead of fetching that for
individual children and adding? That is, why do we need to do the
following:

+ /*
+ * If the relation is a partitioned table, we check it
using reltuples
+ * added up childrens' and changes_since_analyze tracked
by stats collector.

More later...

Thanks,
Amit