ankurdave commented on a change in pull request #32625:
URL: https://github.com/apache/spark/pull/32625#discussion_r637640349
##########
File path: core/src/main/java/org/apache/spark/memory/TaskMemoryManager.java
##########
@@ -202,14 +202,22 @@ public long acquireExecutionMemory(long required,
MemoryConsumer consumer) {
}
}
- // call spill() on itself
- if (got < required) {
+ // Attempt to free up memory by self-spilling.
+ //
+ // When our spill handler releases memory,
`ExecutionMemoryPool#releaseMemory()` will
+ // immediately notify other tasks that memory has been freed, and they
may acquire the
+ // newly-freed memory before we have a chance to do so (SPARK-35486). In
that case, we will
+ // try again in the next loop iteration.
+ while (got < required) {
Review comment:
Good question. The worst case occurs when waiting tasks take all the
memory released by `consumer.spill()`. The specific number of iterations
depends on the behavior of `consumer.spill()`:
- If `consumer.spill()` releases memory promptly when requested to, as all
current implementations do, then this can iterate up to `consumer.getUsed() /
(required - got) + 1` times. In all but the last iteration, `consumer` spills
`required - got` bytes, and in the last iteration, `consumer` spills 0 bytes.
- If `consumer.spill()` trickles out memory, for example by releasing only 1
byte per call, then there can be up to `consumer.getUsed() + 1` iterations. In
all but the last iteration, `consumer` spills 1 byte, and in the last
iteration, `consumer` spills 0 bytes.
This is the same worst-case behavior as the `while
(!sortedConsumers.isEmpty())` loop above.
--
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
