Hi,
On 2023-05-23 13:28:30 -0400, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
> > Could it help to have a mode where the executor shutdown hook checks how much
> > memory is allocated in ExecutorState and warns if its too much?
>
> It'd be very hard to set a limit for what's "too much", since the amount
> of stuff created initially will depend on the plan size.
I was thinking of some limit that should really never be reached outside of a
leak or work_mem based allocations, say 2GB or so.
> In any case I think that the important issue is not how much absolute space,
> but is there per-row leakage. I wonder if we could do something involving
> checking for continued growth after the first retrieved tuple, or something
> like that.
As-is I some nodes do large allocations the query context, but are not
guaranteed to be reached when gathering the first row. So we would still have
to move such large allocations out of ExecutorState.
I think it might be best to go for a combination of these two
heuristics. Store the size of es_query_context after standard_ExecutorStart(),
that would include the allocation of the executor tree itself. Then in
standard_ExecutorEnd(), if the difference in size of ExecutorState is bigger
than some constant *and* is bigger than the initial size by some factor, emit
a warning.
The constant size difference avoids spurious warnings in case of a small plan
that just grows due to a few fmgr lookups or such, the factor takes care of
the plan complexity?
> > Random aside: I've been wondering whether it'd be worth introducing an
> > in-place representation of Bitmap (e.g. if the low bit is set, the low 63 bits
> > are in-place, if unset, it's a pointer).
>
> Why? Unlike Lists, those things are already a single palloc chunk.
We do a fair amount of 8 byte allocations - they have quite a bit of overhead,
even after c6e0fe1f2a0. Not needing allocations for the common case of
bitmapsets with a max member < 63 seems like it could be worth it.
Greetings,
Andres Freund