Re: [HACKERS] CLUSTER command progress monitor

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
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

In response to

Browse pgsql-hackers by date

  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