Re: [HACKERS] CLUSTER command progress monitor

On Thu, Feb 28, 2019 at 11:54 PM Tatsuro Yamada
<yamada.tatsuro@lab.ntt.co.jp> wrote:
> Attached patch is wip patch.

+       <command>CLUSTER</command> and <command>VACUUM FULL</command>,
showing current progress.

and -> or

+   certain commands during command execution.  Currently, the suppoted
+   progress reporting commands are <command>VACUUM</command> and
<command>CLUSTER</command>.

suppoted -> supported

But I'd just say: Currently, the only commands which support progress
reporting are <command>VACUUM</command> and
<command>CLUSTER</command>.

+   Running <command>VACUUM FULL</command> is listed in
<structname>pg_stat_progress_cluster</structname>
+   view because it uses <command>CLUSTER</command> command
internally.  See <xref linkend='cluster-progress-reporting'>.

How about: Running <command>VACUUM FULL</command> is listed in
<structname>pg_stat_progress_cluster</structname> because both
<command>VACUUM FULL</command> and <command>CLUSTER</command> rewrite
the table, while regular <command>VACUUM</command> only modifies it in
place.

+       Current processing command: CLUSTER/VACUUM FULL.

The command that is running.  Either CLUSTER or VACUUM FULL.

+       Current processing phase of cluster/vacuum full.  See <xref
linkend='cluster-phases'> or <xref linkend='vacuum-full-phases'>.

Current processing phase of CLUSTER or VACUUM FULL.

Or maybe better, just abbreviate to: Current processing phase.

+       Scan method of table: index scan/seq scan.

Eh, shouldn't this be gone now?  And likewise for the view definition?

+       OID of the index.

If the table is being scanned using an index, this is the OID of the
index being used; otherwise, it is zero.

+     <entry><structfield>heap_tuples_total</structfield></entry>

Leftovers.  Skipping over the rest of your documentation changes since
it looks like a bunch of things there still need to be updated.

+ pgstat_progress_start_command(PROGRESS_COMMAND_CLUSTER, tableOid);

This now appears inside cluster_rel(), but also vacuum_rel() is still
doing the same thing.  That's wrong.

+ if(OidIsValid(indexOid))

Missing space.  Please pgindent.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company