Skip to content
Closed
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 44 additions & 23 deletions core/src/main/scala/org/apache/spark/status/AppStatusListener.scala
Original file line number Diff line number Diff line change
Expand Up @@ -759,18 +759,10 @@ private[spark] class AppStatusListener(

// Use RDD distribution to update executor memory and disk usage info.
liveRDD.getDistributions().foreach { case (executorId, rddDist) =>
liveExecutors.get(executorId).foreach { exec =>
if (exec.hasMemoryInfo) {
if (storageLevel.useOffHeap) {
exec.usedOffHeap = addDeltaToValue(exec.usedOffHeap, -rddDist.offHeapUsed)
} else {
exec.usedOnHeap = addDeltaToValue(exec.usedOnHeap, -rddDist.onHeapUsed)
}
}
exec.memoryUsed = addDeltaToValue(exec.memoryUsed, -rddDist.memoryUsed)
exec.diskUsed = addDeltaToValue(exec.diskUsed, -rddDist.diskUsed)
maybeUpdate(exec, now)
}
val maybeExec = liveExecutors.get(executorId)
updateExecutorMemoryDiskInfo(maybeExec, storageLevel, -rddDist.memoryUsed,
-rddDist.diskUsed, Option(-rddDist.offHeapUsed), Option(-rddDist.onHeapUsed))
maybeExec.foreach(exec => maybeUpdate(exec, now))
}
}

Expand Down Expand Up @@ -827,6 +819,7 @@ private[spark] class AppStatusListener(
event.blockUpdatedInfo.blockId match {
case block: RDDBlockId => updateRDDBlock(event, block)
case stream: StreamBlockId => updateStreamBlock(event, stream)
case broadcast: BroadcastBlockId => updateBroadcastBlock(event, broadcast)
case _ =>
}
}
Expand Down Expand Up @@ -884,17 +877,7 @@ private[spark] class AppStatusListener(

// Update the executor stats first, since they are used to calculate the free memory
// on tracked RDD distributions.
maybeExec.foreach { exec =>
if (exec.hasMemoryInfo) {
if (storageLevel.useOffHeap) {
exec.usedOffHeap = addDeltaToValue(exec.usedOffHeap, memoryDelta)
} else {
exec.usedOnHeap = addDeltaToValue(exec.usedOnHeap, memoryDelta)
}
}
exec.memoryUsed = addDeltaToValue(exec.memoryUsed, memoryDelta)
exec.diskUsed = addDeltaToValue(exec.diskUsed, diskDelta)
}
updateExecutorMemoryDiskInfo(maybeExec, storageLevel, memoryDelta, diskDelta, None, None)

// Update the block entry in the RDD info, keeping track of the deltas above so that we
// can update the executor information too.
Expand Down Expand Up @@ -995,6 +978,44 @@ private[spark] class AppStatusListener(
}
}

private def updateBroadcastBlock(
event: SparkListenerBlockUpdated,
broadcast: BroadcastBlockId): Unit = {
val now = System.nanoTime()
val executorId = event.blockUpdatedInfo.blockManagerId.executorId
val storageLevel = event.blockUpdatedInfo.storageLevel

// Whether values are being added to or removed from the existing accounting.
val diskDelta = event.blockUpdatedInfo.diskSize * (if (storageLevel.useDisk) 1 else -1)
val memoryDelta = event.blockUpdatedInfo.memSize * (if (storageLevel.useMemory) 1 else -1)

val maybeExec = liveExecutors.get(executorId)
updateExecutorMemoryDiskInfo(maybeExec, storageLevel, memoryDelta, diskDelta, None, None)
maybeExec.foreach(exec => maybeUpdate(exec, now))

}

// update executor memory and disk usage info

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This just repeats the method name. Remove.

private def updateExecutorMemoryDiskInfo(
maybeExec: Option[LiveExecutor],
storageLevel: StorageLevel,
memoryDelta: Long,
diskDelta: Long,
OffHeapDelta: Option[Long],

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Variable names start with lower case.

I'd also avoid the Option here. That causes extra allocations + boxing which this code should avoid.

In the broadcast update you could just repeat memoryDelta as the parameter, since that's what this boils down to...

OnHeapDelta: Option[Long]): Unit = {
maybeExec.foreach { exec =>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move the foreach to the caller. That avoids repeating the foreach, and you could wrap more logic that doesn't need to run when the executor is not found.

if (exec.hasMemoryInfo) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This block exists in a very similar form in two other places. Feels like time to have a helper method.

if (storageLevel.useOffHeap) {
exec.usedOffHeap = addDeltaToValue(exec.usedOffHeap, OffHeapDelta.getOrElse(memoryDelta))
} else {
exec.usedOnHeap = addDeltaToValue(exec.usedOnHeap, OnHeapDelta.getOrElse(memoryDelta))
}
}
exec.memoryUsed = addDeltaToValue(exec.memoryUsed, memoryDelta)
exec.diskUsed = addDeltaToValue(exec.diskUsed, diskDelta)
}
}

private def getOrCreateStage(info: StageInfo): LiveStage = {
val stage = liveStages.computeIfAbsent((info.stageId, info.attemptNumber),
new Function[(Int, Int), LiveStage]() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -938,6 +938,24 @@ class AppStatusListenerSuite extends SparkFunSuite with BeforeAndAfter {
intercept[NoSuchElementException] {
check[StreamBlockData](stream1.name) { _ => () }
}

// Update a BroadcastBlock.
val broadcast1 = BroadcastBlockId(1L)
listener.onBlockUpdated(SparkListenerBlockUpdated(
BlockUpdatedInfo(bm1, broadcast1, level, 1L, 1L)))

check[ExecutorSummaryWrapper](bm1.executorId) { exec =>
assert(exec.info.memoryUsed === 1L)
assert(exec.info.diskUsed === 1L)
}

// Drop a BroadcastBlock.
listener.onBlockUpdated(SparkListenerBlockUpdated(
BlockUpdatedInfo(bm1, broadcast1, StorageLevel.NONE, 1L, 1L)))
check[ExecutorSummaryWrapper](bm1.executorId) { exec =>
assert(exec.info.memoryUsed === 0)
assert(exec.info.diskUsed === 0)
}
}

test("eviction of old data") {
Expand Down