On Thu, Jul 2, 2020 at 7:46 PM Justin Pryzby <pry...@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
