Repository navigation
Fix Memory Backpressure/Spilling Coordination #20715
Description
Activity
- Partition Aware Scheduling: if there are multiple partitions across one or more active queries, we should ensure that we fairly allocate memory according to partitions
- High and Low memory watermarks: we should have two thresholds for our memory limits. The first is the low watermark: when things should start to spill, and high watermark: when we should error out because we are at max memory and there is no way to continue without blowing out memory allocations
- Ability to "wait" for memory to be free: rather than error out immediately if there is no memory, have the ability to wait until memory is available. This would mean that we would need some sort of method to wait until memory is available. However: this is deadlock prone so we need to be careful, or have it as an opt-in. I.e, have the default behaviour to error out immediately, but allow users of datafusion the ability to wait around for a bit
I have some questions:
For point 1, what is the difference between the proposed change and the existing mechanism in the fair pool?
For point 2, currently we only have a single watermark. What are the benefits of introducing two watermarks? I can see that this may make operators less likely to wait for memory as mentioned in point 3, but I’m wondering if there are other considerations as well.Also for point 3, I can confirm this is a bug in the current implementation, and it would be great to see it fixed in the new memory pool implementation.
For point 1, what is the difference between the proposed change and the existing mechanism in the fair pool?
The fair pool does weird things with partitions. Essentially it uses
num_spillto sub-divide spillable consumers into their own buckets, but in my testing this wasn't sufficient. The reason I think being that each consumer has a portion of memory that is spillable and non-spillable, but when registering you have to pick one or the other.For point 2, currently we only have a single watermark. What are the benefits of introducing two watermarks? I can see that this may make operators less likely to wait for memory as mentioned in point 3, but I’m wondering if there are other considerations as well.
That's not entirely true. There is the total amount within a pool, but there is also memory reservation bytes which essentially acts as two watermarks, albeit in a roundabout way.
Reacted by Adrian Garcia BadaraccoThere is the total amount within a pool, but there is also memory reservation bytes which essentially acts as two watermarks, albeit in a roundabout way.
I do kind of agree with this. It feels like there are two watermarks, but it's not fully explicit in the system. This is just a gut feeling but my guess is that making the low/high watermark system explicit and configurable will allow more operators to participate in this and make it easier for users to reason about the tradeoff of making them ~ the same (query might error) vs. making them ~ wide (we might spill when we didn't really need to).
Another way to think about it is that we have two different memory limits: one for spillable operators and one for non-spillable operators. The spill reservation is the space between those two.
Is your feature request related to a problem or challenge?
Right now in Datafusion there are bugs when you are sorting & spilling with multiple partitions. There is even a comment that on the multi level merge that this is not a great solution:
datafusion/datafusion/physical-plan/src/sorts/multi_level_merge.rs
Lines 361 to 362 in d412ba5
There is also some contention between when we should spill and when we are out of memory. Both of these are essentially two limits on the memory cap.
After we fix memory accounting via solutions in #20714, we should be able to adjust the memory pool implementation to allow for more memory coordination.
Describe the solution you'd like
Implement a new "memory coordinator" infrastructure which has the following properties:
I have experimented with all of these style properties in this branch: main...pydantic:datafusion:memory_observations but it was vibe coded to prove the concept (which it does), but probably needs to be broken out into discrete chunks of work and more thoroughly thought out.
Describe alternatives you've considered
No response
Additional context
No response