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


##########
core/src/main/scala/org/apache/spark/executor/TaskMetrics.scala:
##########
@@ -110,9 +112,22 @@ class TaskMetrics private[spark] () extends Serializable {
    * joins. The value of this accumulator should be approximately the sum of 
the peak sizes
    * across all such data structures created in this task. For SQL jobs, this 
only tracks all
    * unsafe operators and ExternalSort.
+   * This is not equal to peakOnHeapExecutionMemory + 
peakOffHeapExecutionMemory
    */
+  // TODO: SPARK-48789: the naming is confusing since this does not really 
reflect the whole
+  //  execution memory. We'd better deprecate this once we have a replacement.
   def peakExecutionMemory: Long = _peakExecutionMemory.sum
 
+  /**
+   * Peak on heap execution memory as tracked by TaskMemoryManager.
+   */
+  def peakOnHeapExecutionMemory: Long = _peakOnHeapExecutionMemory.sum
+
+  /**
+   * Peak off heap execution memory as tracked by TaskMemoryManager.
+   */
+  def peakOffHeapExecutionMemory: Long = _peakOffHeapExecutionMemory.sum

Review Comment:
   Discuss:
   Is it required that `peakExecutionMemory` <= `peakOnHeapExecutionMemory + 
peakOffHeapExecutionMemory` ?
   Any cases where this might get violated ?
   
   I am trying to reason about completeness of these metrics (given we want to 
eventually deprecate the existing one).
   I expect the above to hold, but want to make sure I am not missing anything.
   +CC @JoshRosen 



-- 
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: reviews-unsubscr...@spark.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

Reply via email to