Postgres Pro
Downloads
Home   >   mailing lists

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

From Haribabu Kommi
Subject Re: [HACKERS] ANALYZE command progress checker
Date
Msg-id CAJrrPGesiM6X7A_mHVuraJ0L0ocGFVTq-Ufnw3R1i4QfAjs_wA@mail.gmail.com
Whole thread Raw
In response to Re: [HACKERS] ANALYZE command progress checker  (Michael Paquier <michael.paquier@gmail.com>)
Responses Re: [HACKERS] ANALYZE command progress checker
List pgsql-hackers
Tree view


On Tue, Mar 7, 2017 at 5:01 PM, Michael Paquier <michael.paquier@gmail.com> wrote:

@@ -496,7 +499,6 @@ do_analyze_rel(Relation onerel, int options,
VacuumParams *params,
        numrows = (*acquirefunc) (onerel, elevel,
                                  rows, targrows,
                                  &totalrows, &totaldeadrows);
-
    /*
Useless diff.

+     <entry>
+       <command>ANALYZE</> is currently collecting the sample rows.
+       The sample it reads is taken randomly.Its size depends on
+       the default_statistics_target parameter value.
+     </entry>
This should use a <varname> markup for default_statistics_target.

@@ -203,6 +204,8 @@ analyze_rel(Oid relid, RangeVar *relation, int options,
    if (onerel->rd_rel->relkind == RELKIND_RELATION ||
        onerel->rd_rel->relkind == RELKIND_MATVIEW)
    {
+       pgstat_progress_start_command(PROGRESS_COMMAND_ANALYZE,
+                                               RelationGetRelid(onerel));
It seems to me that the report should begin in do_analyze_rel().

some more comments,

+ /* Report compute heap stats phase */
+ pgstat_progress_update_param(PROGRESS_ANALYZE_PHASE,
+ PROGRESS_ANALYZE_PHASE_COMPUTE_HEAP_STATS);

The above analyze phase is updated inside a for loop, instead just set it above once.

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

Irrespective of whether the index exists on the table or not, the above analyze phase
is set. It is better to set the above phase and index cleanup phase only when there
are indexes on the table.

+ /* Report total number of heap blocks and collectinf sample row phase*/

Typo? collecting?


+ /* 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.


+#define PROGRESS_ANALYZE_PHASE_COLLECT_INH_SAMPLE_ROWS 2

why there is no code added for the phase, any specific reason?


+/* Phases of analyze */

Can be written as following for better understanding, and also
similar like vacuum.

/* Phases of analyze (as advertised via PROGRESS_ANALYZE_PHASE) */


Regards,
Hari Babu
Fujitsu Australia

pgsql-hackers by date:

Previous
From: Amit Langote
Date:
Subject: Re: [HACKERS] WARNING: relcache reference leak: relation "p1" notclosed
Next
From: Masahiko Sawada
Date:
Subject: Re: [HACKERS] GUC for cleanup indexes threshold.