Re: [HACKERS] ANALYZE command progress checker - Mailing list pgsql-hackers

From Haribabu Kommi
Subject Re: [HACKERS] ANALYZE command progress checker
Date
Msg-id CAJrrPGfhVLN=sDKLXinjZyC1QwqQiZdhRABXz+CZpioOCjc_pQ@mail.gmail.com
Whole thread Raw
In response to Re: [HACKERS] ANALYZE command progress checker  (vinayak <Pokale_Vinayak_q3@lab.ntt.co.jp>)
List pgsql-hackers


On Fri, Mar 10, 2017 at 6:46 PM, vinayak <Pokale_Vinayak_q3@lab.ntt.co.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

pgsql-hackers by date:

Previous
From: Stephen Frost
Date:
Subject: Re: [HACKERS] GSOC Introduction / Eliminate O(N^2) scaling fromrw-conflict tracking in serializable transactions
Next
From: George Papadrosou
Date:
Subject: Re: [HACKERS] GSOC - TOAST'ing in slices