Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/649#discussion_r12304125
--- 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 --
Yes I think we should keep the list, since you don't have to worry about
spaces here and there.
For the null thing, I'm trying to understand what it does. If I have a
command Seq("java", "null", "-cp", "some.jar", "SomeClass"), doesn't this get
compiled to "java null -cp some.jar SomeClass"? It seems that the consequences
are undefined if we leave arbitrary "null" strings in there. Maybe I'm
misunderstanding something? Also I didn't realize it was copy and pasted from
ClientBase, which makes this all the stranger.
---
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.
---
