Horton

Re: [HACKERS] CLUSTER command progress monitor

Tatsuro Yamada <tatsuro.yamada.tf@nttcom.co.jp>

View thread View on postgr.es
From: Tatsuro Yamada <tatsuro.yamada.tf@nttcom.co.jp>
To: Alvaro Herrera <alvherre@2ndquadrant.com>
Cc: Robert Haas <robertmhaas@gmail.com>, Rafia Sabih <rafia.pghackers@gmail.com>, Dmitry Dolgov <9erthalion6@gmail.com>, Peter Geoghegan <pg@bowt.ie>, PostgreSQL-development <pgsql-hackers@postgresql.org>, Jeff Janes <jeff.janes@gmail.com>, Thomas Munro <thomas.munro@enterprisedb.com>, Michael Paquier <michael.paquier@gmail.com>
Date: 2019-08-14T02:38:01Z
Lists: pgsql-hackers 
Hi Alvaro and All,

On 2019/08/13 14:40, Tatsuro Yamada wrote:
> Hi Alvaro!
> 
> On 2019/08/02 3:43, Alvaro Herrera wrote:
>> Hmm, I'm trying this out now and I don't see the index_rebuild_count
>> ever go up.  I think it's because the indexes are built using parallel
>> index build ... or maybe it was the table AM changes that moved things
>> around, not sure.  There's a period at the end when the CLUSTER command
>> keeps working, but it's gone from pg_stat_progress_cluster.
> 
> 
> Thanks for your report.
> I'll investigate it. :)


I did "git bisect" and found the commit:

  03f9e5cba0ee1633af4abe734504df50af46fbd8
  Report progress of REINDEX operations

In src/backend/catalog/index.c,
CLUSTER progress reporting increases index_rebuild_count in reindex_relation()
by pgstat_progress_update_param().
However, reindex_relation() calls reindex_index(), and REINDEX progress reporting is existing on the latter function, and it starts pgstat_progress_start_command() pgstat_progress_end_command() for REINDEX progress reporting.
Therefore, CLUSTER progress reporting failed to update index_rebuild_count because
it made a mistake to update the REINDEX's view, I think.

My Idea to fix that is following:

  - Add a target view name parameter to Progress monitor's API

   For example:
   <Before>
     pgstat_progress_update_param(PROGRESS_CLUSTER_INDEX_REBUILD_COUNT, i).

   <After>
     pgstat_progress_update_param(*PROGRESS_CLUSTER_VIEW*, PROGRESS_CLUSTER_INDEX_REBUILD_COUNT, i).

However, I'm not sure whether it is able or not because I haven't read
the code of the API yet.
What do you think about that? :)

Thanks,
Tatsuro Yamada

Commits

  1. Fix progress reporting of CLUSTER / VACUUM FULL

  2. Fix thinko when ending progress report for a backend

  3. Report progress of REINDEX operations

  4. Add progress reporting for CLUSTER and VACUUM FULL.

  5. Un-hide most cascaded-drop details in regression test results.

  6. Fix memory leak in printtup.c.

  7. Remove unused macro