|From:||Tatsuro Yamada <yamada(dot)tatsuro(at)lab(dot)ntt(dot)co(dot)jp>|
|To:||Robert Haas <robertmhaas(at)gmail(dot)com>|
|Cc:||Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Dmitry Dolgov <9erthalion6(at)gmail(dot)com>, Peter Geoghegan <pg(at)bowt(dot)ie>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, Jeff Janes <jeff(dot)janes(at)gmail(dot)com>, Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com>, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>|
|Subject:||Re: [HACKERS] CLUSTER command progress monitor|
|Views:||Raw Message | Whole Thread | Download mbox | Resend email|
On 2019/03/02 4:15, Robert Haas wrote:
> 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.
Thanks for your comments! :)
I revised the code and the document.
> + <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
> suppoted -> supported
> But I'd just say: Currently, the only commands which support progress
> reporting are <command>VACUUM</command> and
I choose the latter. Fixed.
> + Running <command>VACUUM FULL</command> is listed in
> + 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
> + 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.
Fixed as you suggested.
> + Scan method of table: index scan/seq scan.
> Eh, shouldn't this be gone now? And likewise for the view definition?
Fixed. Sorry, It was an oversight.
> + 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.
I agree. Thanks a lot!
I'll divide the patch into two patch such as code and document.
> + 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.
It was an oversight too. I fixed.
> + if(OidIsValid(indexOid))
> Missing space. Please pgindent.
I Will do pgindent later.
Please find attached files. :)
|Next Message||Peter Eisentraut||2019-03-04 10:36:48||Re: unconstify equivalent for volatile|
|Previous Message||Heikki Linnakangas||2019-03-04 10:27:04||Re: GiST VACUUM|