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 |
Date: | 2019-03-04 10:31:21 |
Message-ID: | e3dd092e-05d1-2c96-e7a0-ed7e4d0ae83b@lab.ntt.co.jp |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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
Fixed.
> + 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>.
I choose the latter. Fixed.
> + 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.
Fixed.
> + Current processing command: CLUSTER/VACUUM FULL.
>
> The command that is running. Either CLUSTER or VACUUM FULL.
Fixed.
> + 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.
Fixed.
> + <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.
Fixed.
I Will do pgindent later.
Please find attached files. :)
Thanks,
Tatsuro Yamada
Attachment | Content-Type | Size |
---|---|---|
progress_monitor_for_cluster_command_v7_code.patch | text/x-patch | 10.8 KB |
progress_monitor_for_cluster_command_v7_doc.patch | text/x-patch | 8.2 KB |
From | Date | Subject | |
---|---|---|---|
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 |