Date: Thu, 05 Sep 2024 14:38:09 +0900 (JST)
Message-Id: <20240905.143809.436581948007856565.ishii@postgresql.org>
To: dgrowleyml@gmail.com
Cc: ashutosh.bapat.oss@gmail.com, pgsql-hackers@postgresql.org
Subject: Re: Add memory/disk usage for WindowAgg nodes in EXPLAIN
From: Tatsuo Ishii <ishii@postgresql.org>
In-Reply-To: 
 <CAApHDvoY8cibGcicLV0fNh=9JVx9PANcWvhkdjBnDCc9Quqytg@mail.gmail.com>
References: <20240709.154409.1091516017393224028.ishii@postgresql.org>
	<20240710.183610.1211398886785110571.ishii@postgresql.org>
	<CAApHDvoY8cibGcicLV0fNh=9JVx9PANcWvhkdjBnDCc9Quqytg@mail.gmail.com>
Mime-Version: 1.0
Content-Type: Text/Plain; charset=us-ascii
Content-Transfer-Encoding: 7bit
Archived-At: 
 <https://www.postgresql.org/message-id/20240905.143809.436581948007856565.ishii%40postgresql.org>
Precedence: bulk

Hi,

> On Wed, 10 Jul 2024 at 21:36, Tatsuo Ishii <ishii@postgresql.org> wrote:
>> v2-0005-Add-memory-disk-usage-for-Window-Aggregate-nodes-.patch: This
>> adds memory/disk usage for Window Aggregate nodes in EXPLAIN (ANALYZE)
>> command. Note that if David's proposal
>> https://www.postgresql.org/message-id/CAHoyFK9n-QCXKTUWT_xxtXninSMEv%2BgbJN66-y6prM3f4WkEHw%40mail.gmail.com
>> is committed, this will need to be adjusted.
> 
> Hi,
> 
> I pushed the changes to WindowAgg so as not to call tuplestore_end()
> on every partition.  Can you rebase this patch over that change?
> 
> It would be good to do this in a way that does not add any new state
> to WindowAggState, you can see that I had to shuffle fields around in
> that struct because the next_parition field would have caused the
> struct to become larger. I've not looked closely, but I expect this
> can be done by adding more code to tuplestore_updatemax() to also
> track the disk space used if the current storage has gone to disk. I
> expect the maxSpace field can be used for both, but we'd need another
> bool field to track if the max used was by disk or memory.
> 
> I think the performance of this would also need to be tested as it
> means doing an lseek() on every tuplestore_clear() when we've gone to
> disk. Probably that will be dominated by all the other overheads of a
> tuplestore going to disk (i.e. dumptuples() etc), but it would be good
> to check this. I suggest setting work_mem = 64 and making a test case
> that only just spills to disk. Maybe do a few thousand partitions
> worth of that and see if you can measure any slowdown.

Thank you for the suggestion. I will look into this.

Best reagards,
--
Tatsuo Ishii
SRA OSS K.K.
English: http://www.sraoss.co.jp/index_en/
Japanese:http://www.sraoss.co.jp