On Thu, Feb 20, 2020 at 5:32 PM Amit Langote <amitlangote09@gmail.com> wrote:
> On Thu, Feb 20, 2020 at 4:50 PM Amit Langote <amitlangote09@gmail.com> wrote:
> > * 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.
>
> Oh, it's only adding up children's pg_class.reltuple, not pgstat
> stats. We need to do that because a partitioned table's
> pg_class.reltuples is always 0 and correctly so. Sorry for not
> reading the patch properly.
Having read the relevant diffs again, I think this could be done
without duplicating code too much. You seem to have added the same
logic in two places: do_autovacuum() and table_recheck_autovac().
More importantly, part of the logic of relation_needs_vacanalyze() is
duplicated in both of the aforementioned places, which I think is
unnecessary and undesirable if you consider maintainability. I think
we could just add the logic to compute reltuples for partitioned
tables at the beginning of relation_needs_vacanalyze() and be done. I
have attached a delta patch to show what I mean. Please check and
tell what you think.
Thanks,
Amit