Re: [HACKERS] CLUSTER command progress monitor

At Tue, 19 Mar 2019 11:02:57 +0900, Tatsuro Yamada <yamada.tatsuro@lab.ntt.co.jp> wrote in <dc0dd07c-f185-0cf9-ba54-c5c31f6514f2@lab.ntt.co.jp>
> On 2019/03/19 10:43, Tatsuro Yamada wrote:
> > Hi Rafia!
> > On 2019/03/18 20:42, Rafia Sabih wrote:
> >> On Fri, 8 Mar 2019 at 09:14, Tatsuro Yamada
> >> <yamada.tatsuro@lab.ntt.co.jp> wrote:
> >>> Attached file is rebased patch on current HEAD.
> >>> I changed a status. :)
> >>>
> >>>
> >> Looks like the patch needs a rebase.
> >> I was on the commit fb5806533f9fe0433290d84c9b019399cd69e9c2
> >>
> >> PFA reject file in case you want to have a look.
> > Thanks for testing it. :)
> > I rebased the patch on the current head:
> > f2004f19ed9c9228d3ea2b12379ccb4b9212641f.
> > Please find attached file.
> > Also, I share my test case of progress monitor below.
> 
> 
> Attached patch is a rebased document patch. :)

The monitor view has four columns:

  heap_tuples_scanned
    : used while the "seq scan" and "index scan" phases.

  heap_blks_total, heap_blks_scanned
    : used only while the "seq scan" phase.

  index_rebuild_count:
    : used only while the "rebuilding index" phase.


Couldn't we change the view like the following?

.. |   phase    | heap tuples scanned | progress indicator  | value
..-+------------+---------------------+---------------------+--------
.. | seq scan ..|  2134214            | heap blocks pct     | 23.5%
.. | index sc ..|  5453395            | (null)              |  (null)
.. | sorting  ..|  <the last value>   | (null)              |  (null)
.. | swapping ..|  <the last value>   | (null)              |  (null)
.. | rebuildi ..|  <the last value>   | index count         |  2
.. | performi ..|  <the last value>   | (null)              |  (null)

Only seq scan phase has two kind of progress indicator so if we
choose "heap blks pct" as the only indicator for the phase, it
could be simplified as:

.. |   phase     | progress indicator  | value
..-+-------------+---------------------+--------
.. | seq scan .. | heap blocks pct     | 23.5%
.. | index sc .. | heap tuples scanned | 2398457
.. | sorting  .. | (null)              | (null)
.. | swapping .. | (null)              | (null)
.. | rebuildi .. | index count         | 2
.. | performi .. | (null)              | (null)

A downside of the view is that it looks quite differently from
pg_stat_progress_vacuum. Since I'm not sure it's a good design,
feel free to oppose/reject this.

finish_heap_swap is also called in matview code path but the
function doesn't seem to detect that situation. Is it right
behavior? (I didn't confirm what happens when it is called from
matview refresh path, though.)

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center