Re: ANALYZE command progress checker - Mailing list pgsql-hackers

On 2017/03/30 17:39, Masahiko Sawada wrote:
> On Wed, Mar 29, 2017 at 5:38 PM, vinayak wrote:
>>>> I have updated the patch.
> 
> I reviewed v7 patch.
> 
> When I ran ANALYZE command to the table having 5 millions rows with 3
> child tables, the progress report I got shows the strange result. The
> following result was output after sampled all target rows from child
> tables (i.g, after finished acquire_inherited_sample_rows). I think
> the progress information should report 100% complete at this point.
> 

[...]

> 
> I guess the cause of this is that you always count the number of
> sampled rows in acquire_sample_rows staring from 0 for each child
> table. num_rows_sampled is reset whenever starting collecting sample
> rows.
> Also, even if table has child table, the total number of sampling row
> is not changed. I think that main differences between analyzing on
> normal table and on partitioned table is where we fetch the sample row
> from. So I'm not sure if adding "computing inherited statistics" phase
> is desirable. If you want to report progress of collecting sample rows
> on child table I think that you might want to add the information
> showing which child table is being analyzed.

Two kinds of statistics are collected if the table is a inheritance parent.

First kind considers the table by itself and calculates regular
single-table statistics using rows sampled from the only available heap
(by calling do_analyze_rel() with inh=false).

The second kind are called inheritance statistics, which consider rows
sampled from the whole inheritance hierarchy (by calling do_analyze_rel()
with inh=true).  Note that we are still collecting statistics for the
parent table, so we not really "analyzing" child tables; we just collect
sample rows from them to calculate whole-tree statistics for the root
parent table.  It might still be possible to show the child table being
sampled though.

> --
> pg_stat_progress_analyze.num_target_sample_rows is set while ignoring
> the number of rows the target table has actually. So If the table has
> rows less than target number of rows, the num_rows_sampled never reach
> to num_target_sample_rows.
> 
> --
> @@ -1180,6 +1213,9 @@ acquire_sample_rows(Relation onerel, int elevel,
>                                 }
> 
>                                 samplerows += 1;
> +
> +                               /* Report number of rows sampled so far */
> +
> pgstat_progress_update_param(PROGRESS_ANALYZE_NUM_ROWS_SAMPLED,
> numrows);
>                         }
>                 }
> 
> I think that it's wrong to use numrows variable to report the progress
> of the number of rows sampled. As the following comment mentioned, we
> first save row into rows[] and increment numrows until numrows <
> targrows. And then we could replace stored sample row with new sampled
> row. That is, after collecting "numrows" rows, analyze can still take
> a long time to get and replace the sample row. To computing progress
> of collecting sample row, IMO reporting the number of blocks we
> scanned so far is better than number of sample rows. Thought?

We can report progress in terms of individual blocks only inside
acquire_sample_rows(), which seems undesirable when one thinks that we
will be resetting the target for every child table.  We should have a
global target that considers all child tables in the inheritance
hierarchy, which maybe is possible if we count them beforehand in
acquire_inheritance_sample_rows().  But why not use target sample rows,
which remains the same for both when we're collecting sample rows from one
table and from the whole inheritance hierarchy.  We can keep the count of
already collected rows in a struct that is used across calls for all the
child tables and increment upward from that count when we start collecting
from a new child table.

>>     /*
>>      * The first targrows sample rows are simply copied into the
>>      * reservoir. Then we start replacing tuples in the sample
>>      * until we reach the end of the relation.  This algorithm is
>>      * from Jeff Vitter's paper (see full citation below). It
>>      * works by repeatedly computing the number of tuples to skip
>>      * before selecting a tuple, which replaces a randomly chosen
>>      * element of the reservoir (current set of tuples).  At all
>>      * times the reservoir is a true random sample of the tuples
>>      * we've passed over so far, so when we fall off the end of
>>      * the relation we're done.
>>      */

It seems that we could use samplerows instead of numrows to count the
progress (if we choose to count progress in terms of sample rows collected).

Thanks,
Amit