Re: [HACKERS] CLUSTER command progress monitor

On Thu, Feb 28, 2019 at 11:54 PM Tatsuro Yamada
<yamada(dot)tatsuro(at)lab(dot)ntt(dot)co(dot)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