Date: Fri, 20 Sep 2019 13:49:47 -0300
From: Alvaro Herrera <alvherre@2ndquadrant.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: Robert Haas <robertmhaas@gmail.com>,
	Masahiko Sawada <sawada.mshk@gmail.com>,
	Tatsuro Yamada <tatsuro.yamada.tf@nttcom.co.jp>,
	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>,
	Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
Subject: Re: [HACKERS] CLUSTER command progress monitor
Message-ID: <20190920164947.GA19171@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20190918021031.GK8909@paquier.xyz>
User-Agent: Mutt/1.9.4 (2018-02-28)
Precedence: bulk

On 2019-Sep-18, Michael Paquier wrote:

> On Tue, Sep 17, 2019 at 10:50:22PM -0300, Alvaro Herrera wrote:
> > On 2019-Sep-18, Michael Paquier wrote:
> >> So, with the clock ticking and the release getting close by, what do
> >> we do for this set of issues?  REINDEX, CREATE INDEX and CLUSTER all
> >> try to build indexes and the current infrastructure is not really
> >> adapted to hold all that.  Robert, Alvaro and Peter E, do you have any
> >> comments to offer?
> > 
> > Which part of it is not already fixed?
> 
> I can still see at least two problems.  There is one issue with
> pgstat_progress_update_param() which gets called in reindex_table() 
> for a progress phase of CLUSTER, and this even if
> REINDEXOPT_REPORT_PROGRESS is not set in the options.

(You mean reindex_relation.)

... but that param update is there for CLUSTER, not for REINDEX, so if
we made it dependent on the option to turn on CREATE INDEX progress
updates, it would break CLUSTER progress reporting.  Also, the parameter
being updated is not used by CREATE INDEX, so there's no spurious
change.  I think this ain't broke, and thus it don't need fixin'.

If anything, I would like the CLUSTER report to show index creation
progress, which would go the opposite way.  But that seems a patch for
pg13.

> Also it seems
> to me that the calls to pgstat_progress_start_command() and
> pgstat_progress_end_command() are at incorrect locations for
> reindex_index() and that those should be one level higher on the stack
> to avoid any kind of interactions with another command whose progress
> has already started.

That doesn't work, because the caller doesn't have the OID of the table,
which pgstat_progress_start_command() needs.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services