viirya commented on a change in pull request #1921:
URL: https://github.com/apache/arrow-datafusion/pull/1921#discussion_r819795845
##########
File path: datafusion/src/execution/memory_manager.rs
##########
@@ -340,7 +341,13 @@ impl MemoryManager {
} else if current < min_per_rqt {
// if we cannot acquire at lease 1/2n memory, just wait for
others
// to spill instead spill self frequently with limited total
mem
- self.cv.wait(&mut rqt_current_used);
+ let timeout = self
+ .cv
+ .wait_for(&mut rqt_current_used, Duration::from_secs(5));
Review comment:
Oh, it's interesting. I'm surprised that you can find some related code
quickly, so are you familiar with Spark codebase and also maybe work on it too?
😄
Spark codebase is quite complicated nowadays. Actually I've not read the
code you link to. What in my mind when I looked at `can_grow_directly`, is
where one memory consumer acquires execution memory
([code](https://github.com/apache/spark/blob/master/core/src/main/java/org/apache/spark/memory/TaskMemoryManager.java#L138)).
If the consumer cannot get as much as it needs at first, the manager will
actively ask other consumers (in the same task) to do spilling. No waiting
here. Actually this is better mode in my mind for the consumer model. I
actually plan to make similar changes to this part of datafusion.
For the code you quoted, I just quickly read it. It seems to be for
different purpose. It is used to manage memory pools on executors for tasks. So
it is reasonable that once a task asks for more memory from a memory pool. If
there is no enough memory in the pool and it's in 1/2n (n is number of tasks,
not consumers here) situation, the task will be blocked infinitely until other
tasks free up memory.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
