Thread: Add last_vacuum_index_scans in pg_stat_all_tables

Hi hackers,

I think having number of index scans of the last vacuum in 
pg_stat_all_tables can be helpful. This value shows how efficiently 
vacuums have performed and can be an indicator to increase 
maintenance_work_mem.

It was proposed previously[1], but it was not accepted due to the 
limitation of stats collector. Statistics are now stored in shared 
memory, so we got more rooms to store statistics. I think this 
statistics is still valuable for some people, so I am proposing this 
again.

Best wishes,

-- 
Ken Kato
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION

[1] 
https://www.postgresql.org/message-id/20171010.192616.108347483.horiguchi.kyotaro%40lab.ntt.co.jp

On 2022-Jul-04, Ken Kato wrote:

> I think having number of index scans of the last vacuum in
> pg_stat_all_tables can be helpful. This value shows how efficiently vacuums
> have performed and can be an indicator to increase maintenance_work_mem.

Yeah, this would be a good metric to expose, since it directly tells how
to set autovacuum_work_mem.  I'm not sure that the shape you propose is
correct, though, because each vacuum run would clobber whatever value
was there before.  No other stats counter works that way; they are all
additive.  But I'm not sure that adding the current number each time is
sensible, either, because then the only thing you know is the average of
the last X runs, which doesn't tell you much.

Saving some sort of history would be much more useful, but of course a
lot more work.

> It was proposed previously[1], but it was not accepted due to the limitation
> of stats collector. Statistics are now stored in shared memory, so we got
> more rooms to store statistics. I think this statistics is still valuable
> for some people, so I am proposing this again.

> [1] https://www.postgresql.org/message-id/20171010.192616.108347483.horiguchi.kyotaro%40lab.ntt.co.jp

I read this thread, but what was proposed there is a bunch of metrics
that are not this one.  The discussions there centered about how it
would be unacceptable to incur in the space cost that would be taken by
adding autovacuum-related metrics completely different from the one you
propose.  That debate is now over, so we're clear to proceed.  But we
need to agree on what to add.

-- 
Álvaro Herrera        Breisgau, Deutschland  —  https://www.EnterpriseDB.com/

On Fri, Jul 8, 2022 at 10:47 AM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
> Saving some sort of history would be much more useful, but of course a
> lot more work.

I think that storing a certain amount of history would be very useful,
for lots of reasons. Not just for instrumentation purposes; I envisage
a design where VACUUM itself makes certain decisions based on the
history of each VACUUM operation against the table. The direction that
things have taken suggests a certain amount about the direction that
things are going in, which we should try to influence.

The simplest and best example of how this could help is probably
freezing, and freeze debt. Currently, the visibility map interacts
with vacuum_freeze_min_age in a way that allows unfrozen all-visible
pages to accumulate. These pages won't be frozen until the next
aggressive VACUUM. But there is no fixed relationship between the
number of XIDs consumed by the system (per unit of wallclock time) and
the number of unfrozen all-visible pages (over the same duration). So
we might end up having to freeze an absolutely enormous number of
pages in the eventual aggressive vacuum. We also might not -- it's
really hard to predict, for reasons that just don't make much sense.

There are a few things we could do here, but having a sense of history
seems like the important part. If (say) the table exceeds a certain
size, and the number of all-visible pages grows and grows (without any
freezing taking place), then we should "proactively" freeze at least
some of the unfrozen all-visible pages in earlier VACUUM operations.
In other words, we should (at the very least) spread out the burden of
freezing those pages over time, while being careful to not pay too
much more than we would with the old approach if and when the workload
characteristics change again.

More generally, I think that we should blur the distinction between
aggressive and non-aggressive autovacuum. Sure, we'd still need VACUUM
to "behave aggressively" in some sense, but that could all happen
dynamically, without committing to a particular course of action until
the last moment -- being able to change our minds at the last minute
can be very valuable, even though we probably won't change our minds
too often.

-- 
Peter Geoghegan

On 2022-07-09 03:18, Peter Geoghegan wrote:
> On Fri, Jul 8, 2022 at 10:47 AM Alvaro Herrera 
> <alvherre@alvh.no-ip.org> wrote:
>> Saving some sort of history would be much more useful, but of course a
>> lot more work.

Thank you for the comments!
Yes, having some sort of history would be ideal in this case.
However, I am not sure how to implement those features at this moment, 
so I will take some time to consider.

At the same time, I think having this metrics exposed in the 
pg_stat_all_tables comes in handy when tuning the 
maintenance_work_mem/autovacuume_work_mem even though it shows the value 
of only last vacuum/autovacuum.

Regards,

-- 
Ken Kato
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION

> Regression is failing on all platforms; please correct that and
> resubmit the patch.

Hi,

Thank you for the review!
I fixed it and resubmitting the patch.

Regards,

-- 
Ken Kato
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION

On 2022/09/16 13:23, Ken Kato wrote:
>> Regression is failing on all platforms; please correct that and
>> resubmit the patch.
> 
> Hi,
> 
> Thank you for the review!
> I fixed it and resubmitting the patch.

Could you tell me why the number of index scans should be tracked for
each table? Instead, isn't it enough to have one global counter, to
check whether the current setting of maintenance_work_mem is sufficient
or not? That is, I'm thinking to have something like pg_stat_vacuum view
that reports, for example, the number of vacuum runs, the total
number of index scans, the maximum number of index scans by one
vacuum run, the number of cancellation of vacuum because of
lock conflicts, etc. If so, when these global counters are high or
increasing, we can think that it may worth tuning maintenance_work_mem.

Regards,

-- 
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION

Hi,

On 2022-09-16 13:23:06 +0900, Ken Kato wrote:
> Thank you for the review!
> I fixed it and resubmitting the patch.

cfbot flags that the docs aren't valid:
https://cirrus-ci.com/task/5309377937670144?logs=docs_build#L295
[15:05:39.683] monitoring.sgml:4574: parser error : Opening and ending tag mismatch: entry line 4567 and row
[15:05:39.683]      </row>
[15:05:39.683]            ^


The problem is that you're not closing the <entry>

Greetings,

Andres Freund

On 2022-Sep-16, Fujii Masao wrote:

> Could you tell me why the number of index scans should be tracked for
> each table? Instead, isn't it enough to have one global counter, to
> check whether the current setting of maintenance_work_mem is sufficient
> or not? That is, I'm thinking to have something like pg_stat_vacuum view
> that reports, for example, the number of vacuum runs, the total
> number of index scans, the maximum number of index scans by one
> vacuum run, the number of cancellation of vacuum because of
> lock conflicts, etc. If so, when these global counters are high or
> increasing, we can think that it may worth tuning maintenance_work_mem.

I think that there are going to be cases where some tables in a database
definitely require multiple index scans no matter what; but you
definitely want to know how many occurred for others, not so highly
trafficked tables.  So I *think* a single counter across the whole
database might not be sufficient.

The way I imagine using this (and I haven't operated databases in quite
a while so this may be all wet) is that I would have a report of which
tables have the highest numbers of indexscans, then study the detailed
vacuum reports for those tables as a way to change autovacuum_work_mem.


On the other hand, we have an absolute high cap of 1 GB for autovacuum's
work_mem, and many systems are already using that as the configured
value.  Maybe trying to fine-tune it is a waste of time.  If a 1TB table
says that it had 4 index scans, what are you going to do about it?  It's
a lost cause.  It sounds like we need more code changes so that more
memory can be used; and also changes so that that memory is used more
efficiently.  We had a patch for this, I don't know if that was
committed already.

-- 
Álvaro Herrera        Breisgau, Deutschland  —  https://www.EnterpriseDB.com/
"Ninguna manada de bestias tiene una voz tan horrible como la humana" (Orual)

I applied this patch in my local environment and would like to reviewthe same before 14-October-2022 with some test
data

postgres=# \d pg_stat_all_tables
                        View "pg_catalog.pg_stat_all_tables"
         Column          |           Type           | Collation | Nullable | Default
-------------------------+--------------------------+-----------+----------+---------
 relid                   | oid                      |           |          |
 schemaname              | name                     |           |          |
 relname                 | name                     |           |          |
 seq_scan                | bigint                   |           |          |
 seq_tup_read            | bigint                   |           |          |
 idx_scan                | bigint                   |           |          |
 idx_tup_fetch           | bigint                   |           |          |
 n_tup_ins               | bigint                   |           |          |
 n_tup_upd               | bigint                   |           |          |
 n_tup_del               | bigint                   |           |          |
 n_tup_hot_upd           | bigint                   |           |          |
 n_live_tup              | bigint                   |           |          |
 n_dead_tup              | bigint                   |           |          |
 n_mod_since_analyze     | bigint                   |           |          |
 n_ins_since_vacuum      | bigint                   |           |          |
 last_vacuum             | timestamp with time zone |           |          |
 last_autovacuum         | timestamp with time zone |           |          |
 last_analyze            | timestamp with time zone |           |          |
 last_autoanalyze        | timestamp with time zone |           |          |
 vacuum_count            | bigint                   |           |          |
 autovacuum_count        | bigint                   |           |          |
 analyze_count           | bigint                   |           |          |
 autoanalyze_count       | bigint                   |           |          |
 last_vacuum_index_scans | bigint                   |           |          |

postgres=# select version();
                                                 version
----------------------------------------------------------------------------------------------------------
 PostgreSQL 15beta4 on x86_64-pc-linux-gnu, compiled by gcc (Ubuntu 9.4.0-1ubuntu1~20.04.1) 9.4.0, 64-bit
(1 row)

> The problem is that you're not closing the <entry>

Thank you for the reviews and comments.
I closed the <entry> so that the problem should be fixed now.

Regards,

-- 
Ken Kato
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION

On Tue, 4 Oct 2022 at 10:20, Ken Kato <katouknl@oss.nttdata.com> wrote:
>
> > The problem is that you're not closing the <entry>
>
> Thank you for the reviews and comments.
> I closed the <entry> so that the problem should be fixed now.

The patch does not apply on top of HEAD as in [1], please post a rebased patch:
=== Applying patches on top of PostgreSQL commit ID
e351f85418313e97c203c73181757a007dfda6d0 ===
=== applying patch ./show_index_scans_in_pg_stat_all_tables_v3.patch
patching file src/backend/utils/activity/pgstat_relation.c
Hunk #1 succeeded at 209 (offset 1 line).
Hunk #2 FAILED at 232.
1 out of 2 hunks FAILED -- saving rejects to file
src/backend/utils/activity/pgstat_relation.c.rej
patching file src/include/pgstat.h
Hunk #1 FAILED at 366.
1 out of 2 hunks FAILED -- saving rejects to file src/include/pgstat.h.rej
patching file src/test/regress/expected/rules.out
Hunk #1 succeeded at 1800 (offset 8 lines).
Hunk #2 succeeded at 2145 (offset 11 lines).
Hunk #3 succeeded at 2193 (offset 14 lines).

[1] - http://cfbot.cputube.org/patch_41_3756.log

Regards,
Vignesh