Re: Autovacuum on partitioned table

Hello Amit-san,

Thanks for your comments.

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

> * Names changes_tuples, m_changes_tuples should be changed_tuples and
> m_changed_tuples, respectively?
Yes, I modified it.

> * 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:
>
I modified as follows to apply this feature to only declaretive partitioning.

- if (!inh)
-  pgstat_report_analyze(onerel, totalrows, totaldeadrows,
-         (va_cols == NIL));
+ if (!inh || onerel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
+ pgstat_report_analyze(onerel, totalrows, totaldeadrows,
+        (va_cols == NIL));


> 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.
>
Yes, indeed.  Partitioned tables don't need to vacuum so I added new
checking process for partitioned tables outside relation_needs_vacanalyze().
However, partitioned tables' tabentry->n_dead_tuples are always 0 so
dovacuum is always false.   So I think that checking both auto vacuum
and analyze for partitioned tables doesn't matter.  I merged v3_amit_delta.patch
into the new patch and found minor bug, partitioned table's reltuples is
overwritten with it's classForm->reltuples, so I fixed it.

Also, I think partitioned tables' changes_since_analyze should be reported
only when Autovacuum process.  So I fixed it too.

-- 
Best regards,
Yuzuko Hosoya
NTT Open Source Software Center