Github user tgravescs commented on a diff in the pull request:

    https://github.com/apache/spark/pull/649#discussion_r12299536
  
    --- Diff: 
yarn/common/src/main/scala/org/apache/spark/deploy/yarn/ExecutorRunnableUtil.scala
 ---
    @@ -85,25 +93,25 @@ trait ExecutorRunnableUtil extends Logging {
             }
         */
     
    -    val commands = List[String](
    -      Environment.JAVA_HOME.$() + "/bin/java" +
    -      " -server " +
    +    val commands = Seq(Environment.JAVA_HOME.$() + "/bin/java",
    +      "-server",
           // Kill if OOM is raised - leverage yarn's failure handling to cause 
rescheduling.
           // Not killing the task leaves various aspects of the executor and 
(to some extent) the jvm in
           // an inconsistent state.
           // TODO: If the OOM is not recoverable by rescheduling it on 
different node, then do
           // 'something' to fail job ... akin to blacklisting trackers in 
mapred ?
    -      " -XX:OnOutOfMemoryError='kill %p' " +
    -      JAVA_OPTS +
    -      " org.apache.spark.executor.CoarseGrainedExecutorBackend " +
    -      masterAddress + " " +
    -      slaveId + " " +
    -      hostname + " " +
    -      executorCores +
    -      " 1> " + ApplicationConstants.LOG_DIR_EXPANSION_VAR + "/stdout" +
    -      " 2> " + ApplicationConstants.LOG_DIR_EXPANSION_VAR + "/stderr")
    -
    -    commands
    +      "-XX:OnOutOfMemoryError='kill %p'") ++
    +      JAVA_OPTS ++
    +      Seq("org.apache.spark.executor.CoarseGrainedExecutorBackend",
    +      masterAddress.toString,
    +      slaveId.toString,
    +      hostname.toString,
    +      executorCores.toString,
    +      "1>", ApplicationConstants.LOG_DIR_EXPANSION_VAR + "/stdout",
    +      "2>", ApplicationConstants.LOG_DIR_EXPANSION_VAR + "/stderr")
    +
    +    // TODO: it would be nicer to just make sure there are no null 
commands here
    +    commands.map(s => if (s == null) "null" else s).toList
    --- End diff --
    
    Note, I can take the changes to make it a list out if you would like and 
just make that a separate jira, I just ran into the space issue so thought I 
would make it the same.  Really we should perhaps make a utility function for 
it but we don't have any util classes across client/app master right now.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---

Reply via email to