Horton

Re: Default setting for enable_hashagg_disk

Peter Geoghegan <pg@bowt.ie>

View thread View on postgr.es
From: Peter Geoghegan <pg@bowt.ie>
To: Tomas Vondra <tomas.vondra@2ndquadrant.com>
Cc: Bruce Momjian <bruce@momjian.us>, Robert Haas <robertmhaas@gmail.com>, Andres Freund <andres@anarazel.de>, David Rowley <dgrowleyml@gmail.com>, Jeff Davis <pgsql@j-davis.com>, Justin Pryzby <pryzby@telsasoft.com>, Melanie Plageman <melanieplageman@gmail.com>, "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Date: 2020-06-29T21:46:38Z
Lists: pgsql-hackers, pgsql-docs 
On Mon, Jun 29, 2020 at 2:22 PM Tomas Vondra
<tomas.vondra@2ndquadrant.com> wrote:
> Can you give and example of what you mean by being too permissive or too
> conservative? Do you mean the possibility of unlimited memory usage in
> v12, and strict enforcement in v13?

Yes -- that's all I meant.

> IMO enforcing the work_mem limit (in v13) is right in principle, but I
> do understand the concerns about unexpected regressions compared to v12.

Yeah. Both of these two things are true at the same time.

> I don't understand what you mean by "less memory" when the whole issue
> is significantly exceeding work_mem?

I was just reiterating what I said a few times already: Not using an
in-memory hash aggregate when the amount of memory required is high
but not prohibitively high is penny wise, pound foolish. It's easy to
imagine this actually using up more memory when an entire workload is
considered. This observation does not apply to a system that only ever
has one active connection at a time, but who cares about that?

> I don't think the OOM is the only negative performance here - using a
> lot of memory also forces eviction of data from page cache (although
> writing a lot of temporary files may have similar effect).

True.

> I agree with this, and I'm mostly OK with having hash_mem. In fact, from
> the proposals in this thread I like it the most - as long as it's used
> both during planning and execution. It's a pretty clear solution.

Great.

It's not trivial to write the patch, since there are a few tricky
cases. For example, maybe there is some subtlety in nodeAgg.c with
AGG_MIXED cases. Is there somebody else that knows that code better
than I do that wants to have a go at writing a patch?

-- 
Peter Geoghegan

Commits

  1. Add hash_mem_multiplier GUC.

  2. HashAgg: use better cardinality estimate for recursive spilling.

  3. Remove hashagg_avoid_disk_plan GUC.

  4. Doc fixup for hashagg_avoid_disk_plan GUC.

  5. Rework HashAgg GUCs.

  6. Disk-based Hash Aggregation.

  7. Implement partition-wise grouping/aggregation.

  8. Defer creation of partially-grouped relation until it's needed.