sarutak commented on code in PR #56448:
URL: https://github.com/apache/spark/pull/56448#discussion_r3395551180


##########
resource-managers/yarn/src/main/scala/org/apache/spark/deploy/yarn/config/package.scala:
##########
@@ -348,6 +348,15 @@ package object config extends Logging {
       .stringConf
       .createOptional
 
+  private[spark] val YARN_EXECUTOR_OOM_KILL_ENABLED =
+    ConfigBuilder("spark.yarn.executor.OOMKill.enabled")

Review Comment:
   Considering the naming convention of existing config keys, how about 
`spark.yarn.executor.oomKill.enabled`?



##########
resource-managers/yarn/src/main/scala/org/apache/spark/deploy/yarn/config/package.scala:
##########
@@ -348,6 +348,15 @@ package object config extends Logging {
       .stringConf
       .createOptional
 
+  private[spark] val YARN_EXECUTOR_OOM_KILL_ENABLED =
+    ConfigBuilder("spark.yarn.executor.OOMKill.enabled")
+      .doc("Whether to add `-XX:OnOutOfMemoryError='kill %p'`(or its 
counterpart on Windows) " +

Review Comment:
   It's better to insert a white space between "%p'" and "`(or"



##########
resource-managers/yarn/src/test/scala/org/apache/spark/deploy/yarn/ExecutorRunnableSuite.scala:
##########
@@ -138,4 +139,30 @@ class ExecutorRunnableSuite extends SparkFunSuite with 
PrivateMethodTester {
     commands should contain ("-XX:ActiveProcessorCount=7")
     commands should contain inOrderElementsOf List("--cores", "7")
   }
+
+  test("test executor OnOutOfMemoryError JVM options") {
+    Seq(true, false).foreach { OOMKill =>

Review Comment:
   nit: oomKill is better?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to