Re: [HACKERS] CLUSTER command progress monitor

On Mon, Mar 4, 2019 at 5:38 AM Tatsuro Yamada
<yamada.tatsuro@lab.ntt.co.jp> wrote:
> === Current design ===
>
> CLUSTER command uses Index Scan or Seq Scan when scanning the heap.
> Depending on which one is chosen, the command will proceed in the
> following sequence of phases:
>
>    * Scan method: Seq Scan
>      0. initializing                 (*2)
>      1. seq scanning heap            (*1)
>      3. sorting tuples               (*2)
>      4. writing new heap             (*1)
>      5. swapping relation files      (*2)
>      6. rebuilding index             (*2)
>      7. performing final cleanup     (*2)
>
>    * Scan method: Index Scan
>      0. initializing                 (*2)
>      2. index scanning heap          (*1)
>      5. swapping relation files      (*2)
>      6. rebuilding index             (*2)
>      7. performing final cleanup     (*2)
>
> VACUUM FULL command will proceed in the following sequence of phases:
>
>      1. seq scanning heap            (*1)
>      5. swapping relation files      (*2)
>      6. rebuilding index             (*2)
>      7. performing final cleanup     (*2)
>
> (*1): increasing the value in heap_tuples_scanned column
> (*2): only shows the phase in the phase column

All of that sounds good.

> The view provides the information of CLUSTER command progress details as follows
> # \d pg_stat_progress_cluster
>                View "pg_catalog.pg_stat_progress_cluster"
>            Column           |  Type   | Collation | Nullable | Default
> ---------------------------+---------+-----------+----------+---------
>   pid                       | integer |           |          |
>   datid                     | oid     |           |          |
>   datname                   | name    |           |          |
>   relid                     | oid     |           |          |
>   command                   | text    |           |          |
>   phase                     | text    |           |          |
>   cluster_index_relid       | bigint  |           |          |
>   heap_tuples_scanned       | bigint  |           |          |
>   heap_tuples_vacuumed      | bigint  |           |          |

Still not sure if we need heap_tuples_vacuumed.  We could try to
report heap_blks_scanned and heap_blks_total like we do for VACUUM, if
we're using a Seq Scan.

> === Discussion points ===
>
>   - Progress counter for "3. sorting tuples" phase
>      - Should we add pgstat_progress_update_param() in tuplesort.c like a
>        "trace_sort"?
>        Thanks to Peter Geoghegan for the useful advice!

How would we avoid an abstraction violation?

>   - Progress counter for "6. rebuilding index" phase
>      - Should we add "index_vacuum_count" in the view like a vacuum progress monitor?
>        If yes, I'll add pgstat_progress_update_param() to reindex_relation() of index.c.
>        However, I'm not sure whether it is okay or not.

Doesn't seem unreasonable to me.

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