Re: Report planning memory in EXPLAIN ANALYZE
Andy Fan <zhihui.fan1213@gmail.com>
From: Andy Fan <zhihui.fan1213@gmail.com>
To: Lepikhov Andrei <a.lepikhov@postgrespro.ru>
Cc: Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>,
jian he <jian.universality@gmail.com>, David Rowley <dgrowleyml@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2023-09-22T02:52:12Z
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
Commits
-
Add EXPLAIN (MEMORY) to report planner memory consumption
- 5de890e3610d 17.0 landed
-
Fix explain regression test failure.
- eabba4a3eb71 14.0 cited