Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1qkcNj-00FEn8-BG for pgsql-hackers@arkaria.postgresql.org; Mon, 25 Sep 2023 03:35:35 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.94.2) (envelope-from ) id 1qkcNh-00D4ur-Qn for pgsql-hackers@arkaria.postgresql.org; Mon, 25 Sep 2023 03:35:33 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1qkcNh-00D4uV-DF for pgsql-hackers@lists.postgresql.org; Mon, 25 Sep 2023 03:35:33 +0000 Received: from mail-oo1-xc33.google.com ([2607:f8b0:4864:20::c33]) by magus.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (Exim 4.94.2) (envelope-from ) id 1qkcNe-007PIT-CV for pgsql-hackers@lists.postgresql.org; Mon, 25 Sep 2023 03:35:32 +0000 Received: by mail-oo1-xc33.google.com with SMTP id 006d021491bc7-57b9cb05fa3so1317876eaf.1 for ; Sun, 24 Sep 2023 20:35:29 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1695612928; x=1696217728; darn=lists.postgresql.org; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=NpW1h8xqBnvYWe8ePk74nbCiEsBOeVIEsbKWF0QIVQg=; b=cy+pdVzJhdRSro9/MHvfNyY9JN4XkZwJ2A7XI6Y6m3T3lwk0tLB49GFapAYybGC3aH 7oz5Idqrn+c9+T+5JcsCA1ajAaE7erIDhX3VrJaAKMoPbwssELbQ7SbO56k6eGbIs7Wy gnmzYcGAz0JYjY7xFrzDWHbC+1lPIRE2M1/ssxHwcnqbPL2cjMtRITi2k8RHP5P/SesB CbKnck2iRgfp8I0BJfZiGyjHc8WPH7OvbNexb7fDsJ3moi5clxvD8grvFtL1rJTUIhne 28XrMwvNnwp81ALoTdcSY7ZuhcUbMkAosBjrvpVRFyFDEJ5Q2dWp60W3HQG2HNt/IFot FZXg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1695612928; x=1696217728; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=NpW1h8xqBnvYWe8ePk74nbCiEsBOeVIEsbKWF0QIVQg=; b=mEEFKOSwbPcf6yqd7rvJLtgL2ZI9Db/3BaVlQ5ru8fvfYzOOHXpEWWiGMDlCk5WV3q Wkd2PybMb9U05kRZttNJdT9j2xPWwX8qm+Fi69Vzer1rMZc96M509ZQBDrj/6FYUEbDl csmZvPHd45ECbfF+HLDjwpVOWGXphFJ4yzYnLZbYUkxxfwFXwTsIet3J6Isrd17GdpCH b55PXXJsQDJ8D0t+Xgw9SOjVA+DnjobJ1Yt6Tgj0CTSJxUXTnHAVI8L1nv6zTaL8V+2J v5MwEmRwAA3grmAU67Q00nm0oUpzmfNAHt6LSO5T08xwhqQLotAYx0JXBy8oytwT3aNR QtUg== X-Gm-Message-State: AOJu0Yye0yjWn4MV4ycTB8Nbgn+SlWJqo0ewo+9YLIsqUvobkFFsCoKg RLi+CNld4Gz6PhyEWXC8Y0N+LJ43LsUaJeDPXho= X-Google-Smtp-Source: AGHT+IHEjAspaf8wWW1vGwL9+CE8iThENs0XSjFrOosiS4e8bkp9m8dl8cgXk+zOPSJbemN076Bs22zJ+phMVt6ZfwI= X-Received: by 2002:a4a:2802:0:b0:571:aceb:26d3 with SMTP id h2-20020a4a2802000000b00571aceb26d3mr3582800ooa.4.1695612928277; Sun, 24 Sep 2023 20:35:28 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Andy Fan Date: Mon, 25 Sep 2023 11:35:17 +0800 Message-ID: Subject: Re: Report planning memory in EXPLAIN ANALYZE To: Ashutosh Bapat Cc: Lepikhov Andrei , jian he , David Rowley , PostgreSQL Hackers Content-Type: multipart/alternative; boundary="000000000000444373060626a61d" List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk --000000000000444373060626a61d Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Hi Ashutosh, On Fri, Sep 22, 2023 at 5:56=E2=80=AFPM Ashutosh Bapat wrote: > Hi Andy, > Thanks for your feedback. > > On Fri, Sep 22, 2023 at 8:22=E2=80=AFAM Andy Fan wrote: > > > > 1). The commit message of patch 1 just says how it does but doesn't > > say why it does. After reading through the discussion, I suggest making > > it clearer to others. > > > > ... > > After the planning is done, it may still occupy lots of memory which is > > allocated but not pfree-d. All of these memories can't be used in the > later > > stage. This patch will report such memory usage in order to making some > > future mistakes can be found in an easier way. > > ... > > That's a good summary of how the memory report can be used. Will > include a line about usage in the commit message. > > > > > Planning Memory: used=3D15088 bytes allocated=3D16384 bytes > > > > Word 'Planning' is kind of a time duration, so the 'used' may be the > > 'used' during the duration or 'used' after the duration. Obviously you > > means the later one but it is not surprising others think it in the oth= er > > way. So I'd like to reword the metrics to: > > We report "PLanning Time" hence used "Planning memory". Would > "Planner" be good instead of "Planning"? > I agree "Planner" is better than "Planning" personally. > > > Memory Occupied (Now): Parser: 1k Planner: 10k > > > > 'Now' is a cleaner signal that is a time point rather than a time > > duration. 'Parser' and 'Planner' also show a timeline about the > > important time point. At the same time, it cost very little coding > > effort based on patch 01. Different people may have different > > feelings about these words, I just hope I describe the goal clearly. > > Parsing happens before planning and that memory is not measured by > this patch. May be separately but it's out of scope of this work. > "used" and "allocated" are MemoryContext terms indicating memory > actually used vs memory allocated. Yes, I understand the terms come from MemoryContext, the complex thing here is between time duration vs time point case. Since English is not my native language, so I'm not too confident about insisting on this. So I think we can keep it as it is. > > > > Personally I am pretty like patch 1, we just need to refactor some word= s > > to make everything clearer. > > Thanks. > David challenged something at the begining, but IIUC he also agree the value of patch 01 as the previous statement after discussion. Since the patch is mild itself, so I will mark this commitfest entry as "Ready for committer". Please correct me if anything is wrong. --=20 Best Regards Andy Fan --000000000000444373060626a61d Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Hi Ashutosh,

On Fri, Sep 22, 2023 at 5:56=E2=80=AFPM A= shutosh Bapat <ashutosh.= bapat.oss@gmail.com> wrote:
Hi Andy,
Thanks for your feedback.

On Fri, Sep 22, 2023 at 8:22=E2=80=AFAM Andy Fan <zhihui.fan1213@gmail.com> wr= ote:
>
> 1). The commit message of patch 1 just says how it does but doesn'= t
> say why it does. After reading through the discussion, I suggest makin= g
> it clearer to others.
>
> ...
> After the planning is done, it may still occupy lots of memory which i= s
> allocated but not pfree-d. All of these memories can't be used in = the later
> stage. This patch will report such memory usage in order to making som= e
> future mistakes can be found in an easier way.
> ...

That's a good summary of how the memory report can be used. Will
include a line about usage in the commit message.

>
> Planning Memory: used=3D15088 bytes allocated=3D16384 bytes
>
> Word 'Planning' is kind of a time duration, so the 'used&#= 39; may be the
> 'used' during the duration or 'used' after the duratio= n. Obviously you
> means the later one but it is not surprising others think it in the ot= her
> way. So I'd like to reword the metrics to:

We report "PLanning Time" hence used "Planning memory".= Would
"Planner" be good instead of "Planning"?
=C2=A0
I agree "Planner" is better than "Pl= anning" personally.=C2=A0

>
> Memory Occupied (Now): Parser: 1k Planner: 10k
>
> 'Now' is a cleaner signal that is a time point rather than a t= ime
> duration. 'Parser' and 'Planner' also show a timeline = about the
> important time point. At the same time, it cost very little coding
> effort based on patch 01. Different people may have different
> feelings about these words, I just hope I describe the goal clearly.
Parsing happens before planning and that memory is not measured by
this patch. May be separately but it's out of scope of this work.
"used" and "allocated" are MemoryContext terms indicati= ng memory
actually used vs memory allocated.

Yes, I u= nderstand the terms come=C2=A0from MemoryContext, the complex
thi= ng here is between time duration vs time point case. Since English
is not my native language, so I'm not too confident about insisting o= n this.=C2=A0
So I think we can keep it as it is.

<= /div>
=C2=A0
>
> Personally I am pretty like patch 1, we just need to refactor some wor= ds
> to make everything clearer.

Thanks.

David challenged something at t= he begining,=C2=A0 but IIUC he also agree the
value of patch 01 a= s the previous statement after discussion. Since the patch
is mil= d itself, so I will mark this commitfest entry as "Ready for committer= ".=C2=A0=C2=A0
Please=C2=A0 correct me if anything is wrong.= =C2=A0

-- =
Best= Regards
Andy Fan
--000000000000444373060626a61d--