Message-ID: <59B7D119.2000101@lab.ntt.co.jp>
Date: Tue, 12 Sep 2017 21:20:41 +0900
From: Tatsuro Yamada <yamada.tatsuro@lab.ntt.co.jp>
User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64;
 rv:31.0) Gecko/20100101 Thunderbird/31.1.0
MIME-Version: 1.0
Subject: Re: CLUSTER command progress monitor
References: <59A77072.3090401@lab.ntt.co.jp>
 <CA+TgmobcMQEFvJkEqunpXfj2Uuc21Lt_0X_nJowd9CEebY15GQ@mail.gmail.com>
 <59B5F6C0.1090008@lab.ntt.co.jp>
 <CA+Tgmoa2Huap7teh5RPin+36++GbBKukoQgha5KDhJTJqKeF1g@mail.gmail.com>
In-Reply-To: 
 <CA+Tgmoa2Huap7teh5RPin+36++GbBKukoQgha5KDhJTJqKeF1g@mail.gmail.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 7bit
To: Robert Haas <robertmhaas@gmail.com>
Cc: "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Precedence: bulk
Sender: pgsql-hackers-owner@postgresql.org

On 2017/09/11 23:38, Robert Haas wrote:
> On Sun, Sep 10, 2017 at 10:36 PM, Tatsuro Yamada
> <yamada.tatsuro@lab.ntt.co.jp> wrote:
>> Thanks for the comment.
>>
>> As you know, CLUSTER command uses SEQ SCAN or INDEX SCAN as a scan method by
>> cost estimation. In the case of SEQ SCAN, these two phases not overlap.
>> However, in INDEX SCAN, it overlaps. Therefore I created the phase of "scan
>> heap and write new heap" when INDEX SCAN was selected.
>>
>> I agree that progress reporting for sort is difficult. So it only reports
>> the phase ("sorting tuples") in the current design of progress monitor of
>> cluster.
>> It doesn't report counter of sort.
>
> Doesn't that make it almost useless?  I would guess that scanning the
> heap and writing the new heap would ordinarily account for most of the
> runtime, or at least enough that you're going to want something more
> than just knowing that's the phase you're in.

Hmmm, Should I add a counter in tuplesort.c? (tuplesort_performsort())
I know that external merge sort takes a time than quick sort.
I'll try investigating how to get a counter from external merge sort processing.
Is this the right way?


>>> The patch is getting the value reported as heap_tuples_total from
>>> OldHeap->rd_rel->reltuples.  I think this is pointless: the user can
>>> see that value anyway if they wish.  The point of the progress
>>> counters is to expose things the user couldn't otherwise see.  It's
>>> also not necessarily accurate: it's only an estimate in the best case,
>>> and may be way off if the relation has recently be extended by a large
>>> amount.  I think it's pretty important that we try hard to only report
>>> values that are known to be accurate, because users hate (and mock)
>>> inaccurate progress reports.
>>
>> Do you mean to use the number of rows by using below calculation instead
>> OldHeap->rd_rel->reltuples?
>>
>>   estimate rows = physical table size / average row length
>
> No, I mean don't report it at all.  The caller can do that calculation
> if they wish, without any help from the progress reporting machinery.

I see. I'll remove that column on next patch.


Regards,
Tatsuro Yamada


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers