Re: Report planning memory in EXPLAIN ANALYZE

From: Andy Fan <zhihui(dot)fan1213(at)gmail(dot)com>
To: Lepikhov Andrei <a(dot)lepikhov(at)postgrespro(dot)ru>
Cc: Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com>, jian he <jian(dot)universality(at)gmail(dot)com>, David Rowley <dgrowleyml(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Report planning memory in EXPLAIN ANALYZE
Date: 2023-09-22 02:52:12
Message-ID: CAKU4AWrD_raPLCWknLw7f=ZscVPE2unTYpOwzahNV1=HUMmc_g@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi Ashutosh,

Thanks for the patch!

I think the reason why others are not excited about this is because the
benefit is not clear enough to them after the first glance and plus the
chosen wolds "Planning Memory: used " is still confusing for different
people. I admit it is clear to you absolutely, just not for others. Here
are some minor feedback from me:

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.
...

(a description of how it works just like what you have done in the commit
message).

So if the people read the commit message, they can understand where the
function is used for.

2). at the second level, it would be cool that the user can understand the
metric without reading the commit message.

Planning Memory: used=15088 bytes allocated=16384 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 other
way. So I'd like to reword the metrics to:

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.

About patch 2 & patch 3, since I didn't find a good usage of it, so I
didn't put much effort on it. interesting probably is not a enough
reason for adding it IMO, since there are too many interesting things.

Personally I am pretty like patch 1, we just need to refactor some words
to make everything clearer.

--
Best Regards
Andy Fan

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andy Fan 2023-09-22 03:02:25 Re: Questioning an errcode and message in jsonb.c
Previous Message Hayato Kuroda (Fujitsu) 2023-09-22 02:44:41 RE: CI: Unfamiliar error while testing macOS