From: | Michael Paquier <michael(dot)paquier(at)gmail(dot)com> |
---|---|
To: | Andres Freund <andres(at)anarazel(dot)de> |
Cc: | David Steele <david(at)pgmasters(dot)net>, vinayak <Pokale_Vinayak_q3(at)lab(dot)ntt(dot)co(dot)jp>, David Fetter <david(at)fetter(dot)org>, Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: ANALYZE command progress checker |
Date: | 2017-03-07 06:01:45 |
Message-ID: | CAB7nPqSty907wQCF1kO7_Zy9QbcC6ifqg07dNfx4c5f7dAK28g@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Mon, Mar 6, 2017 at 3:58 PM, Andres Freund <andres(at)anarazel(dot)de> wrote:
> On 2017-03-03 15:33:15 -0500, David Steele wrote:
>> On 3/1/17 1:25 PM, Andres Freund wrote:
>> > On 2017-03-01 10:20:41 -0800, David Fetter wrote:
>> >> On Wed, Mar 01, 2017 at 09:45:40AM -0500, Peter Eisentraut wrote:
>> >>> On 2/28/17 04:24, vinayak wrote:
>> >>>> The view provides the information of analyze command progress details as
>> >>>> follows
>> >>>> postgres=# \d pg_stat_progress_analyze
>> >>>> View "pg_catalog.pg_stat_progress_analyze"
>> >>>
>> >>> Hmm, do we want a separate "progress" system view for every kind of
>> >>> command? What if someone comes up with a progress checker for CREATE
>> >>> INDEX, REINDEX, CLUSTER, etc.?
>> >
>> > I don't think that'd be that bad, otherwise the naming of the fields is
>> > complicated. I guess the alternative (or do both?) would be to to have
>> > a pivoted table, but that'd painful to query. Do you have a better idea?
>> >
>> >
>> >> Some kind of design for progress seems like a good plan. Some ideas:
>> >>
>> >> - System view(s)
>> >>
>> >> This has the advantage of being shown to work at least to a PoC by
>> >> this patch, and is similar to extant system views like
>> >> pg_stat_activity in the sense of capturing a moment in time.
>> >>
>> >> - NOTIFY
>> >>
>> >> Event-driven model as opposed to a polling one. This is
>> >> attractive on efficiency grounds, less so on reliability ones.
>> >>
>> >> - Something added to the wire protocol
>> >>
>> >> More specialized, limits the information to the session where the
>> >> command was issued
>> >>
>> >> - Other things not named here
>> >
>> > We now have a framework for this [1] (currently used by vacuum, but
>> > extensible). The question is about presentation. I'm fairly sure that
>> > we shouldn't just add yet another framework, and I doubt that that's
>> > what's proposed by Peter.
>>
>> I think the idea of a general progress view is very valuable and there
>> are a ton of operations it could be used for: full table scans, index
>> rebuilds, vacuum, copy, etc.
>> However, I feel that this proposal is not flexible enough and comes too
>> late in the release cycle to allow development into something that could
>> be committed.
>
> I'm not following. As I pointed out, we already have this framework?
> This patch is just a short one using that framework?
>
>
>> I propose we move this to the 2017-07 CF so the idea can be more fully
>> developed.
>
> I don't see that being warranted in this case, we're really not talking
> about something complicated:
> doc/src/sgml/monitoring.sgml | 135 +++++++++++++++++++++++++++++++++++
> src/backend/catalog/system_views.sql | 16 ++++
> src/backend/commands/analyze.c | 34 ++++++++
> src/backend/utils/adt/pgstatfuncs.c | 2
> src/include/commands/progress.h | 13 +++
> src/include/pgstat.h | 3
> src/test/regress/expected/rules.out | 18 ++++
> 7 files changed, 219 insertions(+), 2 deletions(-)
> excepting tests and docs, this is very little actual code.
Or 35 lines just for the backend portion, it is hard to something smaller.
@@ -496,7 +499,6 @@ do_analyze_rel(Relation onerel, int options,
VacuumParams *params,
numrows = (*acquirefunc) (onerel, elevel,
rows, targrows,
&totalrows, &totaldeadrows);
-
/*
Useless diff.
+ <entry>
+ <command>ANALYZE</> is currently collecting the sample rows.
+ The sample it reads is taken randomly.Its size depends on
+ the default_statistics_target parameter value.
+ </entry>
This should use a <varname> markup for default_statistics_target.
@@ -203,6 +204,8 @@ analyze_rel(Oid relid, RangeVar *relation, int options,
if (onerel->rd_rel->relkind == RELKIND_RELATION ||
onerel->rd_rel->relkind == RELKIND_MATVIEW)
{
+ pgstat_progress_start_command(PROGRESS_COMMAND_ANALYZE,
+ RelationGetRelid(onerel));
It seems to me that the report should begin in do_analyze_rel().
--
Michael
From | Date | Subject | |
---|---|---|---|
Next Message | Ashutosh Sharma | 2017-03-07 06:33:27 | Re: Parallel seq. plan is not coming against inheritance or partition table |
Previous Message | Amit Kapila | 2017-03-07 05:48:17 | Re: Parallel seq. plan is not coming against inheritance or partition table |