Hi,

On Thu, Sep 5, 2024 at 2:01 PM Alena Rybakina <a.rybak...@postgrespro.ru> wrote:
>
> Hi! Thank you for your review!
>
> On 05.09.2024 15:47, jian he wrote:
>
> On Thu, Sep 5, 2024 at 1:23 AM Alena Rybakina <a.rybak...@postgrespro.ru> 
> wrote:
>
> Hi, all!
>
> I have attached the new version of the code and the diff files
> (minor-vacuum.no-cbot).

Thank you for updating the patches. I've reviewed the 0001 patch and
have two comments.

I think we can split the 0001 patch into two parts: adding
pg_stat_vacuum_tables system views that shows the vacuum statistics
that we are currently collecting such as scanned_pages and
removed_pages, and another one is to add new statistics to collect
such as vacrel->set_all_visible_pages and visibility map updates.

I'm concerned that a pg_stat_vacuum_tables view has some duplicated
statistics that we already collect in different ways. For instance,
total_blks_{read,hit,dirtied,written} are already tracked at
system-level by pg_stat_io, and per-relation block I/O statistics can
be collected using pg_stat_statements. Having duplicated statistics
consumes more memory for pgstat and could confuse users if these
statistics are not consistent. I think it would be better to avoid
collecting duplicated statistics in different places.

Regards,

-- 
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com


Reply via email to