Re: Add mention of execution time memory for enable_partitionwise_* GUCs

From: David Rowley <dgrowleyml(at)gmail(dot)com>
To: Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com>
Cc: Dimitrios Apostolou <jimis(at)gmx(dot)net>, PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Add mention of execution time memory for enable_partitionwise_* GUCs
Date: 2024-07-30 12:08:40
Message-ID: CAApHDvoYFCGJqYV4UmWb3kriDks4WMxSc0tnFzQE2+psHvXrFQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, 30 Jul 2024 at 21:12, Ashutosh Bapat
<ashutosh(dot)bapat(dot)oss(at)gmail(dot)com> wrote:
> Thanks. This looks better. Nitpick
>
> + child partitions. With this setting enabled, the number of executor
> + nodes whose memory usage is restricted by <varname>work_mem</varname>
>
> This sentence appears to say that the memory usage of "all" nodes is
> restricted by work_mem. I think what you want to convey is - nodes,
> whose memory usage is subjected to <varname>work_mem</varname>
> setting, ....

I'm open to improving the sentence but I don't quite follow why
"subjected to" is better than "restricted by". It seems to remove
meaning without saving any words. With "restricted by" we can deduce
that the memory cannot go over work_mem, whereas "subjected to" only
gives us some indication that the memory usage somehow relates to the
work_mem setting and doesn't not mean that the memory used could be >=
work_mem.

> Or break it into two sentences
>
> With this setting enabled, the number of executor nodes appearing in
> the final plan can increase linearly proportional to the number of
> partitions being scanned. Each of those nodes may use upto
> <varname>work_mem</varname> memory. This can ...

... but that's incorrect. This means that all additional nodes that
appear in the plan as a result of enabling the setting can use up to
work_mem. That's not the case as some of the new nodes might be
Nested Loops or Merge Joins and they're not going to use up to
work_mem.

As a compromise, I've dropped the word "executor" and it now just says
"the number of nodes whose memory usage is restricted by work_mem
appearing in the final plan". I'd have used "plan nodes" but it seems
strange to use "plan nodes" then later "in the final plan".

> I note that the work_mem documentation does not talk about executor
> nodes, instead it uses the term "query operations".

I much prefer "nodes" to "operations". If someone started asking me
about "query operations", I'd have to confirm what they meant. An I/O
is an operation that can occur during the execution of a query. Is
that a "query operation"? IMO, the wording you're proposing is less
concise. I'm not a fan of the terminology when talking about plan
nodes. I'd rather see the work_mem docs modified than copy what it
says.

Please have a look at: git grep -B 1 -i -F "plan node" -- doc/*

that seems like a popular term.

David

Attachment Content-Type Size
partitionwise_docs2.patch application/octet-stream 2.3 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Thomas Munro 2024-07-30 12:33:05 Re: PG17beta2: SMGR: inconsistent type for nblocks
Previous Message Andrew Dunstan 2024-07-30 11:59:26 Re: jsonpath: Inconsistency of timestamp_tz() Output