Horton

Re: Default setting for enable_hashagg_disk (hash_mem)

Peter Geoghegan <pg@bowt.ie>

View thread View on postgr.es
From: Peter Geoghegan <pg@bowt.ie>
To: Justin Pryzby <pryzby@telsasoft.com>
Cc: Tomas Vondra <tomas.vondra@2ndquadrant.com>, 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>, Melanie Plageman <melanieplageman@gmail.com>, PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Date: 2020-07-03T03:56:29Z
Lists: pgsql-hackers, pgsql-docs 
On Thu, Jul 2, 2020 at 7:46 PM Justin Pryzby <pryzby@telsasoft.com> wrote:
> Thanks for putting it together, I agree that hash_mem seems to be an obvious
> "escape hatch" that generalizes existing GUCs and independently useful.

It is independently useful. It's a natural consequence of "being
honest" about work_mem and hashing.

> I feel it should same as work_mem, as it's written, and not a multiplier.
>
> And actually I don't think a lower value should be ignored: "mechanism not
> policy".  Do we refuse atypical values of maintenance_work_mem < work_mem ?

I see your point, but AFAIK maintenance_work_mem was not retrofit like
this. It seems different. (Unless we add the -1 behavior, perhaps)

> I assumed that hash_mem would default to -1, which would mean "fall back to
> work_mem".  We'd then advise users to increase it if they have an issue in v13
> with performance of hashes spilled to disk.  (And maybe in other cases, too.)

Yeah, this kind of -1 behavior could make sense.

> I read the argument that hash tables are a better use of RAM than sort.
> However it seems like setting the default to greater than work_mem is a
> separate change than providing the mechanism allowing user to do so.  I guess
> the change in default is intended to mitigate the worst possible behavior
> change someone might experience in v13 hashing, and might be expected to
> improve "out of the box" performance.  I'm not opposed to it, but it's not an
> essential part of the patch.

That's true.

> In nodeHash.c, you missed an underscore:
> +        * Target in-memory hashtable size is hashmem kilobytes.

Got it; thanks.

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