From: | yuzuko <yuzukohosoya(at)gmail(dot)com> |
---|---|
To: | Amit Langote <amitlangote09(at)gmail(dot)com> |
Cc: | Masahiko Sawada <masahiko(dot)sawada(at)2ndquadrant(dot)com>, Laurenz Albe <laurenz(dot)albe(at)cybertec(dot)at>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Greg Stark <stark(at)mit(dot)edu> |
Subject: | Re: Autovacuum on partitioned table |
Date: | 2020-02-21 06:14:05 |
Message-ID: | CAKkQ509MqcJ7eG3DJD7vNMVTrSBV3=88V-KrV4zT0GTw1HfwTA@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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
Attachment | Content-Type | Size |
---|---|---|
v4_autovacuum_on_partitioned_table.patch | application/octet-stream | 15.8 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Craig Ringer | 2020-02-21 06:14:21 | Re: Add PGURI env var for passing connection string to psql in Docker |
Previous Message | Michael Paquier | 2020-02-21 06:07:12 | Re: [Patch] Make pg_checksums skip foreign tablespace directories |