pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Tatsuo Ishii <ishii@postgresql.org>
To: dgrowleyml@gmail.com
Cc: pgsql-hackers@postgresql.org
Subject: Re: Add memory/disk usage for WindowAgg nodes in EXPLAIN
Date: Tue, 09 Jul 2024 11:44:12 +0900 (JST)
Message-ID: <20240709.114412.199303920149670049.ishii@postgresql.org> (raw)
In-Reply-To: <CAApHDvpQ3Duc7+c5=-7b60Mq-B6A=eGJT=gEb2V=J4c15-KLdw@mail.gmail.com>
References: <20240706.202254.89740021795421286.ishii@postgresql.org>
	<CAApHDvpQ3Duc7+c5=-7b60Mq-B6A=eGJT=gEb2V=J4c15-KLdw@mail.gmail.com>

> On Sat, 6 Jul 2024 at 23:23, Tatsuo Ishii <ishii@postgresql.org> wrote:
>> So I wanted to Add memory/disk usage for WindowAgg. Patch attached.
> 
> Thanks for working on that.

Thank you for the infrastructure you created in tuplestore.c and explain.c.

BTW, it seems these executor nodes (other than Materialize and Window
Aggregate node) use tuplestore for their own purpose.

CTE Scan
Recursive Union
Table Function Scan

I have already implemented that for CTE Scan. Do you think other two
nodes are worth to add the information? I think for consistency sake,
it will better to add the info Recursive Union and Table Function
Scan.

>> Since WindowAgg node could create multiple tuplestore for each Window
>> partition, we need to track each tuplestore storage usage so that the
>> maximum storage usage is determined. For this purpose I added new
>> fields to the WindowAggState.
> 
> I'd recently been looking at the code that recreates the tuplestore
> for each partition and thought we could do a bit better. In [1], I
> proposed a patch to make this better.
> 
> If you based your patch on [1], maybe a better way of doing this is
> having tuplestore.c track the maximum space used on disk in an extra
> field which is updated with tuplestore_clear().  It's probably ok to
> update a new field called maxDiskSpace in tuplestore_clear() if
> state->status != TSS_INMEM. If the tuplestore went to disk then an
> extra call to BufFileSize() isn't going to be noticeable, even in
> cases where we only just went over work_mem. You could then adjust
> tuplestore_space_used() to look at maxDiskSpace and return that value
> if it's larger than BufFileSize(state->myfile) and state->maxSpace.
> You could check if maxDiskSpace == 0 to determine if the tuplestore
> has ever gone to disk. tuplestore_storage_type_name() would also need
> to check maxDiskSpace and return "Disk" if that's non-zero.

Thank you for the suggestion. Yes, I noticed [1] and once it is
committed, I will start to study tuplestore.c in this direction.

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





view thread (53+ messages)  latest in thread

Message-ID: <20240709.114412.199303920149670049.ishii@postgresql.org>
Permalink:  ../20240709.114412.199303920149670049.ishii@postgresql.org/
Also on:    postgresql.org/message-id/20240709.114412.199303920149670049.ishii@postgresql.org

 · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-hackers@postgresql.org
  Cc: ishii@postgresql.org, dgrowleyml@gmail.com
  Subject: Re: Add memory/disk usage for WindowAgg nodes in EXPLAIN
  In-Reply-To: <20240709.114412.199303920149670049.ishii@postgresql.org>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox