[SPARK-12149] [Web UI] Executor UI improvement suggestions - Color UI #10154
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -33,11 +33,13 @@ private[ui] case class ExecutorSummaryInfo( | |
| rddBlocks: Int, | ||
| memoryUsed: Long, | ||
| diskUsed: Long, | ||
| totalCores: Int, | ||
| activeTasks: Int, | ||
| failedTasks: Int, | ||
| completedTasks: Int, | ||
| totalTasks: Int, | ||
| totalDuration: Long, | ||
| totalGCTime: Long, | ||
| totalInputBytes: Long, | ||
| totalShuffleRead: Long, | ||
| totalShuffleWrite: Long, | ||
|
|
@@ -117,6 +119,32 @@ private[ui] class ExecutorsPage( | |
| val maximumMemory = info.maxMemory | ||
| val memoryUsed = info.memoryUsed | ||
| val diskUsed = info.diskUsed | ||
|
|
||
| // Determine Color Opacity from 0.5-1 | ||
| // activeTasks range from 0 to all cores | ||
| val activeTasksAlpha = | ||
| if (info.totalCores > 0) { | ||
| (info.activeTasks.toDouble / info.totalCores) * 0.5 + 0.5 | ||
|
Contributor
There was a problem hiding this comment. So this isn't taking into account that a task could use more then one core via the spark.task.cpus configuration.
Member
Author
There was a problem hiding this comment. I didn't know about that, currently testing a change that replaces totalCores with maxTasks (= totalCores / spark.task.cpus) in my implementation. We also have the option of not shading the color on active tasks at all.
Member
Author
There was a problem hiding this comment. Just updated it to look at spark.task.cpus |
||
| } else { | ||
| 1 | ||
| } | ||
| // failedTasks range max at 10% failure, alpha max = 1 | ||
| // completedTasks range ignores 90% of tasks | ||
| val (failedTasksAlpha, completedTasksAlpha) = | ||
| if (info.totalTasks > 0) { | ||
| (math.min(10 * info.failedTasks.toDouble / info.totalTasks, 1) * 0.5 + 0.5, | ||
| math.max(((10 * info.completedTasks.toDouble / info.totalTasks) - 9) * 0.5 + 0.5, 0.5)) | ||
| } else { | ||
| (1, 1) | ||
| } | ||
| // totalDuration range from 0 to 50% GC time, alpha max = 1 | ||
| val totalDurationAlpha = | ||
| if (info.totalDuration > 0) { | ||
| math.min(info.totalGCTime.toDouble / info.totalDuration + 0.5, 1) | ||
| } else { | ||
| 1 | ||
| } | ||
|
|
||
| <tr> | ||
| <td>{info.id}</td> | ||
| <td>{info.hostPort}</td> | ||
|
|
@@ -128,11 +156,36 @@ private[ui] class ExecutorsPage( | |
| <td sorttable_customkey={diskUsed.toString}> | ||
| {Utils.bytesToString(diskUsed)} | ||
| </td> | ||
| <td style={ | ||
| if (info.activeTasks > 0) { | ||
| "background:hsla(120, 100%, 25%, " + activeTasksAlpha + ");color:white" | ||
|
Member
There was a problem hiding this comment. Can the
Member
Author
There was a problem hiding this comment. You are correct, it does not work in this syntax, I tired. |
||
| } else { | ||
| "" | ||
| } | ||
| }>{info.activeTasks}</td> | ||
| <td style={ | ||
| if (info.failedTasks > 0) { | ||
| "background:hsla(0, 100%, 50%, " + failedTasksAlpha + ");color:white" | ||
|
Member
There was a problem hiding this comment. Another nit but if alpha is in 0.0 - 0.1 would it be clearer to write "1.0" instead of "100%" for HSL?
Member
Author
There was a problem hiding this comment. That is actually how HSLa is designed and was it seemed to be an area on contention when it was first defined.
Member
There was a problem hiding this comment. Oh so you have to express some as % and some as a fraction? that seems odd. If consistency is at all possible that'd be clearer I think.
Member
Author
There was a problem hiding this comment. I agree, but thats just how HSLa is |
||
| } else { | ||
| "" | ||
| } | ||
| }>{info.failedTasks}</td> | ||
| <td style={ | ||
| if (info.completedTasks > 0) { | ||
| "background:hsla(240, 100%, 50%, " + completedTasksAlpha + ");color:white" | ||
| } else { | ||
| "" | ||
| } | ||
| }>{info.completedTasks}</td> | ||
| <td>{info.totalTasks}</td> | ||
| <td sorttable_customkey={info.totalDuration.toString} style={ | ||
| // Red if GC time over 10% | ||
| if (10 * info.totalGCTime > info.totalDuration) { | ||
| "background:hsla(0, 100%, 50%, " + totalDurationAlpha + ");color:white" | ||
| } else { | ||
| "" | ||
| } | ||
| }> | ||
| {Utils.msDurationToString(info.totalDuration)} | ||
| </td> | ||
| <td sorttable_customkey={info.totalInputBytes.toString}> | ||
|
|
@@ -184,11 +237,13 @@ private[spark] object ExecutorsPage { | |
| val memUsed = status.memUsed | ||
| val maxMem = status.maxMem | ||
| val diskUsed = status.diskUsed | ||
| val totalCores = listener.executorToTotalCores.getOrElse(execId, 0) | ||
| val activeTasks = listener.executorToTasksActive.getOrElse(execId, 0) | ||
| val failedTasks = listener.executorToTasksFailed.getOrElse(execId, 0) | ||
| val completedTasks = listener.executorToTasksComplete.getOrElse(execId, 0) | ||
| val totalTasks = activeTasks + failedTasks + completedTasks | ||
| val totalDuration = listener.executorToDuration.getOrElse(execId, 0L) | ||
| val totalGCTime = listener.executorToJvmGCTime.getOrElse(execId, 0L) | ||
| val totalInputBytes = listener.executorToInputBytes.getOrElse(execId, 0L) | ||
| val totalShuffleRead = listener.executorToShuffleRead.getOrElse(execId, 0L) | ||
| val totalShuffleWrite = listener.executorToShuffleWrite.getOrElse(execId, 0L) | ||
|
|
@@ -200,11 +255,13 @@ private[spark] object ExecutorsPage { | |
| rddBlocks, | ||
| memUsed, | ||
| diskUsed, | ||
| totalCores, | ||
| activeTasks, | ||
| failedTasks, | ||
| completedTasks, | ||
| totalTasks, | ||
| totalDuration, | ||
| totalGCTime, | ||
| totalInputBytes, | ||
| totalShuffleRead, | ||
| totalShuffleWrite, | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -44,10 +44,12 @@ private[ui] class ExecutorsTab(parent: SparkUI) extends SparkUITab(parent, "exec | |
| */ | ||
| @DeveloperApi | ||
| class ExecutorsListener(storageStatusListener: StorageStatusListener) extends SparkListener { | ||
| val executorToTotalCores = HashMap[String, Int]() | ||
| val executorToTasksActive = HashMap[String, Int]() | ||
| val executorToTasksComplete = HashMap[String, Int]() | ||
| val executorToTasksFailed = HashMap[String, Int]() | ||
| val executorToDuration = HashMap[String, Long]() | ||
| val executorToJvmGCTime = HashMap[String, Long]() | ||
| val executorToInputBytes = HashMap[String, Long]() | ||
| val executorToInputRecords = HashMap[String, Long]() | ||
| val executorToOutputBytes = HashMap[String, Long]() | ||
|
|
@@ -62,6 +64,7 @@ class ExecutorsListener(storageStatusListener: StorageStatusListener) extends Sp | |
| override def onExecutorAdded(executorAdded: SparkListenerExecutorAdded): Unit = synchronized { | ||
| val eid = executorAdded.executorId | ||
| executorToLogUrls(eid) = executorAdded.executorInfo.logUrlMap | ||
| executorToTotalCores(eid) = executorAdded.executorInfo.totalCores | ||
|
Contributor
There was a problem hiding this comment. so this might have issues on other cluster manager other then yarn.. see the discussion at #2980
Member
Author
There was a problem hiding this comment. I looked into the differences between that case and this and the values are coming from different places. In this case the totalCores value is given as a param to ExecutorInfo which is called directly by the Standalone (local) and Mesos backends. |
||
| executorIdToData(eid) = ExecutorUIData(executorAdded.time) | ||
| } | ||
|
|
||
|
|
@@ -131,6 +134,7 @@ class ExecutorsListener(storageStatusListener: StorageStatusListener) extends Sp | |
| executorToShuffleWrite(eid) = | ||
| executorToShuffleWrite.getOrElse(eid, 0L) + shuffleWrite.shuffleBytesWritten | ||
| } | ||
| executorToJvmGCTime(eid) = executorToJvmGCTime.getOrElse(eid, 0L) + metrics.jvmGCTime | ||
| } | ||
| } | ||
| } | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So the comment for this case class says it isn't used anymore - do we really need to update it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did some checks, MiMa exclude is still needed even without this change and this change did not affect any code, therefore I've removed the change.