On Tue, Dec 31, 2024 at 6:07 PM Tomas Vondra <tomas@vondra.me> wrote:
>
> So I decided to revisit the three patches from 2019. Attached are
> rebased and cleaned up versions. A couple comments on each one:
>
>
> 1) v20241231-adjust-limit-0001-Account-for-batch-files-in-ha.patch
>
> I believe this is the way to go, for now. The basic idea is to keep the
> overall behavior, but "relax" the memory limit as the number of batches
> increases to minimize the total memory use.
>
> This may seem a bit weird, but as the number of batches grows there's no
> way to not violate the limit. And the current code simply ignores this
> and allocates arbitrary amounts of memory.
I'm just catching up on this thread and haven't read all the mails
yet. I started with looking at the patches in the first email and got
a bit confused.
In this patch (v20241231-adjust-limit-0001-Account-for-batch-files-in-ha.patch),
I see that you've started accounting for the spill files in
hashtable->spaceUsed -- in the same way that is done for the tuples in
the hashtable. I know the other memory contexts (hashCxt and batchCxt)
in hashtable aren't appropriate for figuring out spaceUsed, but I was
wondering if the hashtable->spillCxt accurately reflects how much
memory is being used for these spill files at one time? Perhaps it
doesn't make sense to use this, but when we added it in 8c4040edf45, I
thought we might one day be able to use it for determining peak space
usage. Or perhaps you are imagining it only be used for observability?
- Melanie
