Re: ANALYZE command progress checker

On Fri, Mar 10, 2017 at 6:46 PM, vinayak <Pokale_Vinayak_q3(at)lab(dot)ntt(dot)co(dot)jp>
wrote:

>
> + /* Report total number of heap blocks and collectinf sample row phase*/
> + initprog_val[0] = PROGRESS_ANALYZE_PHASE_COLLECT_HEAP_SAMPLE_ROWS;
> + initprog_val[1] = totalblocks;
> + pgstat_progress_update_multi_param(2, initprog_index, initprog_val);
>
> acquire_sample_rows function is called from acquire_inherited_sample_rows
> function, so adding the phase in that function will provide wrong info.
>
> I agree with you.
>
>
> +#define PROGRESS_ANALYZE_PHASE_COLLECT_INH_SAMPLE_ROWS 2
>
> why there is no code added for the phase, any specific reason?
>
> I am thinking how to report this phase. Do you have any suggestion?
>

Thanks for the update patch.

Actually the analyze operation is done in two stages for the relation that
contains child relations,
1. For parent relation
2. All child relations

So the progress with start each time for the above two stages. This needs
to clearly mentioned in the docs.

The acquire_sample_rows function collects statistics of the relation
that is provided, updating the analyze details like number of rows and etc
works fine for a single relation, but if it contains many child relations,
the
data will be misleading.

Apart from the above, some more comments.

+ /* Report compute index stats phase */
+ pgstat_progress_update_param(PROGRESS_ANALYZE_PHASE,
+ PROGRESS_ANALYZE_PHASE_COMPUTE_INDEX_STATS);

The above block of the code should be set when it actually doing the
compute_index_stats.

+ /* Report that we are now doing index cleanup */
+ pgstat_progress_update_param(PROGRESS_ANALYZE_PHASE,
+ PROGRESS_ANALYZE_PHASE_INDEX_CLEANUP);

The above code block should be inside if (!(options & VACOPT_VACUUM))
block.

Regards,
Hari Babu
Fujitsu Australia