Response to Andrew's review

markhamstra · Jul 7, 2014 · d9ab5a9 · d9ab5a9
1 parent 7752449
commit d9ab5a9
Show file tree

Hide file tree

Showing 4 changed files with 25 additions and 25 deletions.
diff --git a/core/src/main/scala/org/apache/spark/ui/ToolTips.scala b/core/src/main/scala/org/apache/spark/ui/ToolTips.scala
@@ -17,7 +17,7 @@
 
 package org.apache.spark.ui
 
-object ToolTips {
+private[spark] object ToolTips {
   val SCHEDULER_DELAY =
     """Scheduler delay includes time to ship the task from the scheduler to
        the executor, and time the time to send a message from the executor to the scheduler stating

diff --git a/core/src/main/scala/org/apache/spark/ui/UIUtils.scala b/core/src/main/scala/org/apache/spark/ui/UIUtils.scala
@@ -140,16 +140,17 @@ private[spark] object UIUtils extends Logging {
 
   def prependBaseUri(basePath: String = "", resource: String = "") = uiRoot + basePath + resource
 
-  val commonHeaderNodes =
-      <meta http-equiv="Content-type" content="text/html; charset=utf-8" />
-      <link rel="stylesheet" href={prependBaseUri("/static/bootstrap.min.css")}
-            type="text/css" />
-      <link rel="stylesheet" href={prependBaseUri("/static/webui.css")}
-            type="text/css" />
-      <script src={prependBaseUri("/static/sorttable.js")} ></script>
-      <script src={prependBaseUri("/static/jquery-1.11.1.min.js")}></script>
-      <script src={prependBaseUri("/static/bootstrap-tooltip.js")}></script>
-      <script src={prependBaseUri("/static/initialize-tooltips.js")}></script>
+  val commonHeaderNodes = {
+    <meta http-equiv="Content-type" content="text/html; charset=utf-8" />
+    <link rel="stylesheet" href={prependBaseUri("/static/bootstrap.min.css")}
+          type="text/css" />
+    <link rel="stylesheet" href={prependBaseUri("/static/webui.css")}
+          type="text/css" />
+    <script src={prependBaseUri("/static/sorttable.js")} ></script>
+    <script src={prependBaseUri("/static/jquery-1.11.1.min.js")}></script>
+    <script src={prependBaseUri("/static/bootstrap-tooltip.js")}></script>
+    <script src={prependBaseUri("/static/initialize-tooltips.js")}></script>
+  }
 
   /** Returns a spark page with correctly formatted headers */
   def headerSparkPage(

diff --git a/core/src/main/scala/org/apache/spark/ui/exec/ExecutorsPage.scala b/core/src/main/scala/org/apache/spark/ui/exec/ExecutorsPage.scala
@@ -25,7 +25,8 @@ import org.apache.spark.storage.StorageLevel
 import org.apache.spark.ui.{ToolTips, UIUtils, WebUIPage}
 import org.apache.spark.util.Utils
 
-private case class ExecutorInfo(
+/** Summary information about an executor to display in the UI. */
+private case class ExecutorSummaryInfo(
     id: String,
     hostPort: String,
     rddBlocks: Int,
@@ -67,17 +68,12 @@ private[ui] class ExecutorsPage(parent: ExecutorsTab) extends WebUIPage("") {
           <th>Complete Tasks</th>
           <th>Total Tasks</th>
           <th>Task Time</th>
+          <th><span data-toggle="tooltip" title={ToolTips.INPUT}>Input</span></th>
+          <th><span data-toggle="tooltip" title={ToolTips.SHUFFLE_READ}>Shuffle Read</span></th>
           <th>
-            <span data-toggle="tooltip" title={ToolTips.INPUT}>
-              Input
-            </span>
-          </th>
-          <th>
-            <span data-toggle="tooltip" title={ToolTips.SHUFFLE_READ}>
-              Shuffle Read
-            </span>
-          </th>
-          <th>
+            <!-- Place the shuffle write tooltip on the left (rather than the default position
+              of on top) because the shuffle write column is the last column on the right side and
+              the tooltip is wider than the column, so it doesn't fit on top. -->
             <span data-toggle="tooltip" data-placement="left" title={ToolTips.SHUFFLE_WRITE}>
               Shuffle Write
             </span>
@@ -110,7 +106,7 @@ private[ui] class ExecutorsPage(parent: ExecutorsTab) extends WebUIPage("") {
   }
 
   /** Render an HTML row representing an executor */
-  private def execRow(info: ExecutorInfo): Seq[Node] = {
+  private def execRow(info: ExecutorSummaryInfo): Seq[Node] = {
     val maximumMemory = info.maxMemory
     val memoryUsed = info.memoryUsed
     val diskUsed = info.diskUsed
@@ -145,7 +141,7 @@ private[ui] class ExecutorsPage(parent: ExecutorsTab) extends WebUIPage("") {
   }
 
   /** Represent an executor's info as a map given a storage status index */
-  private def getExecInfo(statusId: Int): ExecutorInfo = {
+  private def getExecInfo(statusId: Int): ExecutorSummaryInfo = {
     val status = listener.storageStatusList(statusId)
     val execId = status.blockManagerId.executorId
     val hostPort = status.blockManagerId.hostPort
@@ -164,7 +160,7 @@ private[ui] class ExecutorsPage(parent: ExecutorsTab) extends WebUIPage("") {
     val totalShuffleRead = listener.executorToShuffleRead.getOrElse(execId, 0L)
     val totalShuffleWrite = listener.executorToShuffleWrite.getOrElse(execId, 0L)
 
-    new ExecutorInfo(
+    new ExecutorSummaryInfo(
       execId,
       hostPort,
       rddBlocks,

diff --git a/core/src/main/scala/org/apache/spark/ui/jobs/StageTable.scala b/core/src/main/scala/org/apache/spark/ui/jobs/StageTable.scala
@@ -46,6 +46,9 @@ private[ui] class StageTableBase(
     <th><span data-toggle="tooltip" title={ToolTips.INPUT}>Input</span></th>
     <th><span data-toggle="tooltip" title={ToolTips.SHUFFLE_READ}>Shuffle Read</span></th>
     <th>
+      <!-- Place the shuffle write tooltip on the left (rather than the default position
+        of on top) because the shuffle write column is the last column on the right side and
+        the tooltip is wider than the column, so it doesn't fit on top. -->
       <span data-toggle="tooltip" data-placement="left" title={ToolTips.SHUFFLE_WRITE}>
         Shuffle Write
       </span>