From: | Tomas Vondra <tomas(at)vondra(dot)me> |
---|---|
To: | Melanie Plageman <melanieplageman(at)gmail(dot)com> |
Cc: | PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Adjusting hash join memory limit to handle batch explosion |
Date: | 2025-02-18 15:44:05 |
Message-ID: | 310eeb10-1c75-4361-966a-33fa12970e6e@vondra.me |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Here is an updated patch series, with improved comments etc.
1) v20250218-0001-Reduce-the-impact-of-hashjoin-batch-explos.patch
I believe this bit is ready to go - I've removed the GUC (which was
meant only for development), reworded and cleaned up many of the
comments, etc. It would be good if someone could read through those
again. I plan to get this pushed soon.
2) v20250218-0002-Postpone-hashtable-growth-instead-of-disab.patch
I'm not quite sure about this part yet. While I think this issue can
happen in practice (as demonstrated by reproducers in 0003), I don't
recall any reports - so maybe it's so unlikely it's artificial?
If we assume the issue not purely theoretical, I think the patch is a
reasonable way to handle it. But I'm not sure what to do about running
out of hash bits (if anything).
As we're doubling nbatch values, at some point we run out of bits in the
hash, i.e. we start adding just "0" bits. With the current growEnabled
flag we simply set growEnabled=false, effectively disabling further
nbatch increases. But with "soft" disable we might retry adding more
batches - which won't do anything, but it's a bit pointless. Do you
think the patch needs to handle this case explicitly?
ISTM this issue is largely theoretical, or could be ignored thanks to
the "balancing" patch 0001. Because as we increase the batch size, that
makes the "fast nbatch growth" not an issue. So maybe once 0001 gets in,
the right solution is to get rid of growEnabled entirely?
After all, the patch does almost exactly the same thing as 0001 - it
just increases batch size, to delay the next nbatch doubling.
AFAIK the only piece we'd lose by getting rid of growEnabled is the
handling of using all hash bits.
regards
--
Tomas Vondra
Attachment | Content-Type | Size |
---|---|---|
v20250218-0001-Reduce-the-impact-of-hashjoin-batch-explos.patch | text/x-patch | 8.8 KB |
v20250218-0002-Postpone-hashtable-growth-instead-of-disab.patch | text/x-patch | 6.0 KB |
v20250218-0003-hashjoin-patch-tests.patch | text/x-patch | 691.9 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2025-02-18 15:52:04 | Re: Confine vacuum skip logic to lazy_scan_skip |
Previous Message | Tom Lane | 2025-02-18 15:43:40 | Re: Why does exec_simple_query requires 2 snapshots |