Github user JoshRosen commented on a diff in the pull request:
https://github.com/apache/spark/pull/10170#discussion_r47038991
--- Diff:
core/src/test/scala/org/apache/spark/memory/UnifiedMemoryManagerSuite.scala ---
@@ -154,32 +177,34 @@ class UnifiedMemoryManagerSuite extends
MemoryManagerSuite with PrivateMethodTes
assert(mm.acquireExecutionMemory(800L, taskAttemptId,
MemoryMode.ON_HEAP) === 800L)
assert(mm.executionMemoryUsed === 800L)
assert(mm.storageMemoryUsed === 0L)
- assertEnsureFreeSpaceNotCalled(ms)
+ assertEvictBlocksToFreeSpaceNotCalled(ms)
// Storage should not be able to evict execution
assert(mm.acquireStorageMemory(dummyBlock, 100L, evictedBlocks))
assert(mm.executionMemoryUsed === 800L)
assert(mm.storageMemoryUsed === 100L)
- assertEnsureFreeSpaceCalled(ms, 100L)
+ assertEvictBlocksToFreeSpaceNotCalled(ms)
assert(!mm.acquireStorageMemory(dummyBlock, 250L, evictedBlocks))
assert(mm.executionMemoryUsed === 800L)
assert(mm.storageMemoryUsed === 100L)
- assertEnsureFreeSpaceCalled(ms, 250L)
+ assertEvictBlocksToFreeSpaceCalled(ms, 150L) // try to evict blocks ...
+ assert(evictedBlocks.isEmpty) // ... but don't evict since evicting
will not be sufficient
mm.releaseExecutionMemory(maxMemory, taskAttemptId, MemoryMode.ON_HEAP)
mm.releaseStorageMemory(maxMemory)
// Acquire some execution memory again, but this time keep it within
the execution region
assert(mm.acquireExecutionMemory(200L, taskAttemptId,
MemoryMode.ON_HEAP) === 200L)
assert(mm.executionMemoryUsed === 200L)
assert(mm.storageMemoryUsed === 0L)
- assertEnsureFreeSpaceNotCalled(ms)
+ assertEvictBlocksToFreeSpaceNotCalled(ms)
// Storage should still not be able to evict execution
assert(mm.acquireStorageMemory(dummyBlock, 750L, evictedBlocks))
assert(mm.executionMemoryUsed === 200L)
assert(mm.storageMemoryUsed === 750L)
- assertEnsureFreeSpaceCalled(ms, 750L)
+ assertEvictBlocksToFreeSpaceNotCalled(ms) // since there was 800 bytes
free
assert(!mm.acquireStorageMemory(dummyBlock, 850L, evictedBlocks))
assert(mm.executionMemoryUsed === 200L)
assert(mm.storageMemoryUsed === 750L)
- assertEnsureFreeSpaceCalled(ms, 850L)
+ assertEvictBlocksToFreeSpaceCalled(ms, 800L) // try to evict blocks...
--- End diff --
Yep, this was addressed by changing `maxMemory` to `maxStorageMemory` in
the check at the start of `acquireStorageMemory`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
