JoshRosen commented on code in PR #47192:
URL: https://github.com/apache/spark/pull/47192#discussion_r1667100668


##########
core/src/test/scala/org/apache/spark/memory/MemoryManagerSuite.scala:
##########
@@ -335,6 +335,28 @@ private[memory] trait MemoryManagerSuite extends 
SparkFunSuite {
     tMemManager.releaseExecutionMemory(500L, c)
     assert(tMemManager.getMemoryConsumptionForThisTask === 0L)
   }
+
+  test("task peak execution memory usage") {
+    val memoryManager = createMemoryManager(
+      maxOnHeapExecutionMemory = 1000L,
+      maxOffHeapExecutionMemory = 1000L)
+
+    val tMemManager = new TaskMemoryManager(memoryManager, 1)
+    val offHeapConsumer = new TestMemoryConsumer(tMemManager, 
MemoryMode.OFF_HEAP)
+    val onHeapConsumer = new TestMemoryConsumer(tMemManager, 
MemoryMode.ON_HEAP)
+
+    val result1 = Future {

Review Comment:
   Do we actually need to use Futures in this test case? I see that many of the 
existing nearby test cases are also doing this, but I'm wondering whether 
perhaps those existing tests might have just copied it from some other test. If 
we don't actually require asynchrony in this test case, maybe we could 
eliminate it for the sake of simplicity (and perhaps test flakiness avoidance)?



-- 
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]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to