Review feedback fixes.

rxin · May 16, 2015 · 346f4ea · 346f4ea
1 parent 629c1dc
commit 346f4ea
Show file tree

Hide file tree

Showing 5 changed files with 43 additions and 51 deletions.
diff --git a/core/src/main/scala/org/apache/spark/SparkContext.scala b/core/src/main/scala/org/apache/spark/SparkContext.scala
@@ -27,7 +27,6 @@ import java.util.concurrent.atomic.{AtomicReference, AtomicBoolean, AtomicIntege
 import java.util.UUID.randomUUID
 
 import scala.collection.{Map, Set}
-
 import scala.collection.JavaConversions._
 import scala.collection.generic.Growable
 import scala.collection.mutable.HashMap
@@ -57,7 +56,8 @@ import org.apache.spark.partial.{ApproximateEvaluator, PartialResult}
 import org.apache.spark.rdd._
 import org.apache.spark.rpc.{RpcAddress, RpcEndpointRef}
 import org.apache.spark.scheduler._
-import org.apache.spark.scheduler.cluster.{CoarseGrainedSchedulerBackend, SparkDeploySchedulerBackend, SimrSchedulerBackend}
+import org.apache.spark.scheduler.cluster.{CoarseGrainedSchedulerBackend,
+  SparkDeploySchedulerBackend, SimrSchedulerBackend}
 import org.apache.spark.scheduler.cluster.mesos.{CoarseMesosSchedulerBackend, MesosSchedulerBackend}
 import org.apache.spark.scheduler.local.LocalBackend
 import org.apache.spark.storage._

diff --git a/core/src/main/scala/org/apache/spark/scheduler/SchedulerBackend.scala b/core/src/main/scala/org/apache/spark/scheduler/SchedulerBackend.scala
@@ -52,7 +52,7 @@ private[spark] trait SchedulerBackend {
   /**
    * Get the URLs for the driver logs. These URLs are used to display the links in the UI
    * Executors tab for the driver.
-   * @return The urls to the logs of the driver
+   * @return Map containing the log names and their respective URLs
    */
   def getDriverLogUrls: Option[Map[String, String]] = None
 

diff --git a/core/src/main/scala/org/apache/spark/ui/exec/ExecutorsTab.scala b/core/src/main/scala/org/apache/spark/ui/exec/ExecutorsTab.scala
@@ -79,7 +79,7 @@ class ExecutorsListener(storageStatusListener: StorageStatusListener) extends Sp
         s.blockManagerId.executorId == SparkContext.LEGACY_DRIVER_IDENTIFIER ||
         s.blockManagerId.executorId == SparkContext.DRIVER_IDENTIFIER
       }
-      storageStatus.foreach(s => executorToLogUrls(s.blockManagerId.executorId) = logs.toMap)
+      storageStatus.foreach { s => executorToLogUrls(s.blockManagerId.executorId) = logs.toMap }
     }
   }
 

diff --git a/yarn/src/main/scala/org/apache/spark/deploy/yarn/YarnSparkHadoopUtil.scala b/yarn/src/main/scala/org/apache/spark/deploy/yarn/YarnSparkHadoopUtil.scala
@@ -33,7 +33,7 @@ import org.apache.hadoop.security.UserGroupInformation
 import org.apache.hadoop.yarn.conf.YarnConfiguration
 import org.apache.hadoop.yarn.api.ApplicationConstants
 import org.apache.hadoop.yarn.api.ApplicationConstants.Environment
-import org.apache.hadoop.yarn.api.records.{ContainerId, Priority, ApplicationAccessType}
+import org.apache.hadoop.yarn.api.records.{ApplicationAccessType, ContainerId, Priority}
 import org.apache.hadoop.yarn.util.ConverterUtils
 
 import org.apache.spark.deploy.SparkHadoopUtil

diff --git a/yarn/src/main/scala/org/apache/spark/scheduler/cluster/YarnClusterSchedulerBackend.scala b/yarn/src/main/scala/org/apache/spark/scheduler/cluster/YarnClusterSchedulerBackend.scala
@@ -66,57 +66,49 @@ private[spark] class YarnClusterSchedulerBackend(
   override def getDriverLogUrls: Option[Map[String, String]] = {
     var yarnClientOpt: Option[YarnClient] = None
     var driverLogs: Option[Map[String, String]] = None
-    val appId: String = applicationId()
-    if (appId == null) {
-      logError("Application Id not set. Cannot get Driver Log URLs")
-      None
-    } else {
-      try {
-        val yarnConf = new YarnConfiguration(sc.hadoopConfiguration)
-        val containerId = YarnSparkHadoopUtil.get.getContainerId
+    try {
+      val yarnConf = new YarnConfiguration(sc.hadoopConfiguration)
+      val containerId = YarnSparkHadoopUtil.get.getContainerId
+      yarnClientOpt = Some(YarnClient.createYarnClient())
+      yarnClientOpt.foreach { yarnClient =>
+        yarnClient.init(yarnConf)
+        yarnClient.start()
+        val addresses =
+          NetworkInterface.getNetworkInterfaces.asScala
+            .flatMap(_.getInetAddresses.asScala)
+            .toSeq
 
-        yarnClientOpt = Some(YarnClient.createYarnClient())
-
-        yarnClientOpt.foreach { yarnClient =>
-          yarnClient.init(yarnConf)
-          yarnClient.start()
-          val addresses =
-            NetworkInterface.getNetworkInterfaces.asScala
-              .flatMap(_.getInetAddresses.asScala)
-              .toSeq
-
-          val nodeReport =
-            yarnClient.getNodeReports(NodeState.RUNNING).asScala.find { x =>
-              val host = x.getNodeId.getHost
-              addresses.exists { address =>
-                address.getHostAddress == host ||
-                  address.getHostName == host ||
-                  address.getCanonicalHostName == host
-              }
+        val nodeReport =
+          yarnClient.getNodeReports(NodeState.RUNNING).asScala.find { x =>
+            val host = x.getNodeId.getHost
+            addresses.exists { address =>
+              address.getHostAddress == host ||
+                address.getHostName == host ||
+                address.getCanonicalHostName == host
             }
-          nodeReport.foreach { report =>
-            val httpAddress = report.getHttpAddress
-            // lookup appropriate http scheme for container log urls
-            val yarnHttpPolicy = yarnConf.get(
-              YarnConfiguration.YARN_HTTP_POLICY_KEY,
-              YarnConfiguration.YARN_HTTP_POLICY_DEFAULT
-            )
-            val user = Utils.getCurrentUserName()
-            val httpScheme = if (yarnHttpPolicy == "HTTPS_ONLY") "https://" else "http://"
-            val baseUrl = s"$httpScheme$httpAddress/node/containerlogs/$containerId/$user"
-            logInfo(s"Base URL for logs: $baseUrl")
-            driverLogs = Some(
-              Map("stderr" -> s"$baseUrl/stderr?start=0", "stdout" -> s"$baseUrl/stdout?start=0"))
           }
+        nodeReport.foreach { report =>
+          val httpAddress = report.getHttpAddress
+          // lookup appropriate http scheme for container log urls
+          val yarnHttpPolicy = yarnConf.get(
+            YarnConfiguration.YARN_HTTP_POLICY_KEY,
+            YarnConfiguration.YARN_HTTP_POLICY_DEFAULT
+          )
+          val user = Utils.getCurrentUserName()
+          val httpScheme = if (yarnHttpPolicy == "HTTPS_ONLY") "https://" else "http://"
+          val baseUrl = s"$httpScheme$httpAddress/node/containerlogs/$containerId/$user"
+          logInfo(s"Base URL for logs: $baseUrl")
+          driverLogs = Some(
+            Map("stderr" -> s"$baseUrl/stderr?start=0", "stdout" -> s"$baseUrl/stdout?start=0"))
         }
-      } catch {
-        case e: Exception =>
-          logInfo("Node Report API is not available in the version of YARN being used, so AM" +
-            " logs link will not appear in application UI", e)
-      } finally {
-        yarnClientOpt.foreach(_.close())
       }
-      driverLogs
+    } catch {
+      case e: Exception =>
+        logInfo("Node Report API is not available in the version of YARN being used, so AM" +
+          " logs link will not appear in application UI", e)
+    } finally {
+      yarnClientOpt.foreach(_.close())
     }
+    driverLogs
   }
 }