Repository navigation
Resources exhausted errors are confusing return the biggest memory consumers. #11523
Description
Is your feature request related to a problem or challenge?
When asking for more memory via the memory reservation, the error returned from the underlying memory pool focuses on that specific request. As a result, when debugging a resource exhausted error, we get an error message that looks something like:
Failed to allocate additional 795696944 bytes for RepartitionExec[1]
with 3182787776 bytes already allocated -
maximum available is 63296515
This^^ error is about what next incremental request failed to get more memory, and not about what is using the most memory. As a result, additional dev time has to be spent to (a) at best, track down the actual high memory consumer and (b) at worst, wasted time chasing the wrong memory consumer.
Describe the solution you'd like
One possible solution is to have the error message returned the Top K memory consumers.
Describe alternatives you've considered
No response
Additional context
No response
Activity
[-]Resources exhuasted errors should return the biggest memory consumers.[/-][+]Resources exhuasted errors are confusing return the biggest memory consumers.[/+]alamb commentedon Jul 17, 2024
I agree this message is quite confusing
There is a related idea here I think #6934
I think @westonpace mentioned something similar when they were debugging OOM issues too
comphead commentedon Jul 17, 2024
The message is quite confusing and we can probably make it better, mentioning requested, used, allocated and remaining mem. But I'm not sure if there is an easy way to what takes the mem, as memory reservation has no idea about the caller just the mem pool name.
wiedld commentedon Jul 17, 2024
Well timed comment @comphead . I was just linking a previous (closed) PR which started down the path of tracking memory consumers (for the purposes of debug dumping). I'm not proposing that as the solution per se, rather giving us an idea of prior work.
alamb commentedon Jul 22, 2024
I believe @wiedld plans to look at this one later this week. We have seen it with our customers and it is quite confusing.
alamb commentedon Jul 31, 2024
@wiedld has an initial PR to add
TrackedMemoryPoolin #11665Here is my proposal for what remains to close this issue
TrackConsumersPoolas the default memory poolThoughts on error messages
Message today:
@wiedld 's proposal on https://github.com/apache/datafusion/pull/11665/files#r1693465283
I think the new proposal is better as it is clearer what is going on
Thoughts on changing the default pool
The the default is set here:
datafusion/datafusion/execution/src/runtime_env.rs
Line 211 in 89677ae
I believe we should change the default pool in DataFusion to be a
TrackedMemoryPool<GreedyMemoryPool>otherwise the error messages when a memory limit will continue to have the issues that are described in this tickets.The only potential issue with changing the default is that the tracking has some additional runtime overhead -- therefore we should run benchmark tests to ensure there is no performance regression.
Also, a nice part of @wiedld 's
TrackedMemoryPooldesign is that someone finds the overhead to be too large, they can change back to the existingGreedyMemoryPool[-]Resources exhuasted errors are confusing return the biggest memory consumers.[/-][+]Resources exhausted errors are confusing return the biggest memory consumers.[/+]