[SPARK-27900][CORE][K8s] Add `spark.driver.killOnOOMError` flag in cluster mode #26161

skonto · 2019-10-18T09:58:06Z

What changes were proposed in this pull request?

Adds a flag to make the driver exit in case of an oom error in cluster mode (enabled by default).
Adds integration tests for the K8s manager.
Adds verbose flag support within the driver's container.

Why are the changes needed?

See for details Spark-27900. In addition, this follows the discussion here: #24796. Without this pods on K8s will keep running although Spark has failed. In addition current behavior creates a problem to the Spark Operator and any other operator as it cannot detect failure at the K8s level.

How was this patch tested?

Manually by launching SparkPi with a large number 100000000 which leads to an oom due to the large number of tasks allocated.

$kubectl logs spark-pi-driver -n spark
19/10/18 09:47:02 INFO KubernetesClusterSchedulerBackend: SchedulerBackend is ready for scheduling beginning after reached minRegisteredResourcesRatio: 0.8
19/10/18 09:47:02 INFO BlockManagerMasterEndpoint: Registering block manager 172.17.0.6:33435 with 413.9 MiB RAM, BlockManagerId(2, 172.17.0.6, 33435, None)
#
# java.lang.OutOfMemoryError: Java heap space
# -XX:OnOutOfMemoryError="kill -9 %p"
#   Executing /bin/sh -c "kill -9 14"...

Also there are two integration tests for the K8s resource manager. Testing might be needed for the other managers.

skonto · 2019-10-18T09:59:58Z

@holdenk @erikerlandson pls review.

SparkQA · 2019-10-18T10:42:56Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/17254/

SparkQA · 2019-10-18T11:03:14Z

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/17254/

SparkQA · 2019-10-18T12:30:50Z

Test build #112270 has finished for PR 26161 at commit 5505083.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

skonto · 2019-10-21T19:26:00Z

@erikerlandson can I get a merge? Gentle ping :)

skonto · 2019-10-23T12:45:48Z

@holdenk gentle ping.

dongjoon-hyun · 2019-10-24T21:11:13Z

Retest this please.

dongjoon-hyun · 2019-10-24T21:12:50Z

core/src/main/scala/org/apache/spark/deploy/SparkSubmit.scala

+    // set oom error handling in cluster mode
+    if (sparkConf.get(KILL_ON_OOM_ERROR) && deployMode == CLUSTER) {
+      val driverJavaOptions = sparkConf.getOption(SparkLauncher.DRIVER_EXTRA_JAVA_OPTIONS)
+        .map( _ + " ")


nit. .map( _ + " ") -> .map(_ + " ")

dongjoon-hyun · 2019-10-24T21:17:24Z

core/src/main/scala/org/apache/spark/deploy/SparkSubmit.scala

+    if (sparkConf.get(KILL_ON_OOM_ERROR) && deployMode == CLUSTER) {
+      val driverJavaOptions = sparkConf.getOption(SparkLauncher.DRIVER_EXTRA_JAVA_OPTIONS)
+        .map( _ + " ")
+        .getOrElse("") + "-XX:OnOutOfMemoryError=\"kill -9 %p\""


Since this is a general PR for SparkSubmit, does this work on Windows?
ExitOnOutOfMemoryError will be a better choice, @skonto .

Maybe, I lost some context since this is the 3rd try for this.

I actually think we should stick w/ OnOutOfMemoryError, because ExitOnOutOfMemoryError was not present till java 8u92 (discussed earlier here: #24796 (comment)). I don't think we specify a minimum version within java 8, so we might have to stick with this.

but yeah, we probably have to make sure it doesn't do anything too weird on windows (does spark actually run in anything other than local mode on windows?)

For Windows, I prefer to use JVM option. Personally, I don't think Apache Spark 3.0.0 will be used on JDK 8u91 or older. Apache Spark 3.0.0 starts a new age of JDK11. 😄

dongjoon-hyun · 2019-10-24T21:17:55Z

core/src/main/scala/org/apache/spark/internal/config/package.scala


+  private[spark] val KILL_ON_OOM_ERROR = ConfigBuilder("spark.driver.killOnOOMError")
+    .doc("Whether to kill the driver on an oom error in cluster mode.")
+    .booleanConf.createWithDefault(true)


Please split this into two lines like the other conf.

And this should be false by default to avoid the behavior change.

actually I think we agreed to go ahead and change the default
#25229 (comment)

3.0 is a good chance to do this

and I agree about splitting to two lines to match the style for other confs

Thanks. Got it. In that case, I'm +1 for the true by default.

BTW, @squito . Do you think we need to add a migration guide for this behavior change?

Also, cc @gatorsmile .

Yes we agreed to change the default, we want to fail by default.

dongjoon-hyun · 2019-10-24T21:18:50Z

core/src/main/scala/org/apache/spark/internal/config/package.scala

    .doc("The amount of memory used per page in bytes")
    .bytesConf(ByteUnit.BYTE)
    .createOptional
-


Let's not touch the irrelevant place.

dongjoon-hyun · 2019-10-24T21:19:57Z

resource-managers/kubernetes/docker/src/main/dockerfiles/spark/entrypoint.sh

+      $VERBOSE_FLAG
      --conf "spark.driver.bindAddress=$SPARK_DRIVER_BIND_ADDRESS"
      --deploy-mode client
      "$@"


This file contains orthogonal changes. Please refer this file. You can file a new JIRA issue for this.

@dongjoon-hyun this is required by the tests, so it could be just DEBUG mode, disabled by default (as it is now). It is not meant to be another feature and does no harm. I want to debug the verbose output so tests in K8s can get the java option values set in the driver. Is there another way to trigger this (beyond writing a main that prints them and adding it to Spark examples package)?

dongjoon-hyun · 2019-10-24T21:21:21Z

...ation-tests/src/test/scala/org/apache/spark/deploy/k8s/integrationtest/BasicTestsSuite.scala

    runSparkRemoteCheckAndVerifyCompletion(appArgs = Array(REMOTE_PAGE_RANK_FILE_NAME))
  }
+
+  test("Run SparkPi without the default exit on OOM error flag", k8sTestTag) {


If you are suggesting this PR as a bug fix, you need to add SPARK-27900 prefix to the newly added test case names.

Ok I can do that.

dongjoon-hyun · 2019-10-24T21:21:50Z

...ation-tests/src/test/scala/org/apache/spark/deploy/k8s/integrationtest/BasicTestsSuite.scala

    "https://storage.googleapis.com/spark-k8s-integration-tests/files/pagerank_data.txt"
  val REMOTE_PAGE_RANK_FILE_NAME = "pagerank_data.txt"
 }
+


Let's not add this new line.

dongjoon-hyun · 2019-10-24T21:23:58Z

...ation-tests/src/test/scala/org/apache/spark/deploy/k8s/integrationtest/BasicTestsSuite.scala

+  test("Run SparkPi without the default exit on OOM error flag", k8sTestTag) {
+    sparkAppConf
+      .set("spark.driver.extraJavaOptions", "-Dspark.test.foo=spark.test.bar")
+      .set("spark.kubernetes.driverEnv.DRIVER_VERBOSE", "true")


Could you try SPARK_PRINT_LAUNCH_COMMAND instead of new DRIVER_VERBOSE?

dongjoon-hyun

Hi, @skonto .
I know this is the 3rd try for you. I also had many experiences like this in the community. Anyway, this PR looks better because it is now aiming narrowly driver surgically. I left a few comments. Could you update the PR?

SparkQA · 2019-10-24T22:01:03Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/17589/

SparkQA · 2019-10-24T22:31:42Z

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/17589/

SparkQA · 2019-10-24T23:42:06Z

Test build #112624 has finished for PR 26161 at commit 5505083.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

skonto · 2019-10-27T19:27:09Z

@dongjoon-hyun no problem I will update it.

skonto · 2019-11-08T20:45:01Z

Ok I will fix this over the weekend, sorry for the delay.

skonto · 2020-04-13T20:10:46Z

I will update the PR.

dongjoon-hyun · 2020-04-13T20:31:15Z

Thank you, @skonto !

skonto · 2020-04-14T13:15:45Z

I am back.

github-actions · 2020-07-24T00:32:37Z

We're closing this PR because it hasn't been updated in a while. This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable.
If you'd like to revive this PR, please reopen it and ask a committer to remove the Stale tag!

skonto · 2020-11-23T09:26:15Z

@dongjoon-hyun I will revive this unless it is fixed already. I guess I need to create a new PR correct?

dimon222 · 2024-05-10T03:29:57Z

Was this ever fixed?

holdenk · 2024-05-13T20:29:42Z

I don't think so. We manually set kill on OOM in our config.

fix oom

5505083

skonto mentioned this pull request Oct 18, 2019

[SPARK-27900][K8s] Add jvm oom flag #25229

Closed

dongjoon-hyun added the SPARK CORE label Oct 18, 2019

dongjoon-hyun added the KUBERNETES label Oct 24, 2019

dongjoon-hyun changed the title ~~[SPARK-27900][K8s] Add jvm oom flag in cluster mode~~ [SPARK-27900][K8s] Add spark.driver.killOnOOMError flag in cluster mode Oct 24, 2019

dongjoon-hyun reviewed Oct 24, 2019

View reviewed changes

dongjoon-hyun changed the title ~~[SPARK-27900][K8s] Add spark.driver.killOnOOMError flag in cluster mode~~ [SPARK-27900][CORE][K8s] Add spark.driver.killOnOOMError flag in cluster mode Oct 25, 2019

github-actions bot added the Stale label Jul 24, 2020

github-actions bot closed this Jul 25, 2020

[SPARK-27900][CORE][K8s] Add spark.driver.killOnOOMError flag in cluster mode #26161

[SPARK-27900][CORE][K8s] Add spark.driver.killOnOOMError flag in cluster mode #26161

Uh oh!

Conversation

skonto commented Oct 18, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

Why are the changes needed?

How was this patch tested?

Uh oh!

skonto commented Oct 18, 2019

Uh oh!

SparkQA commented Oct 18, 2019

Uh oh!

SparkQA commented Oct 18, 2019

Uh oh!

SparkQA commented Oct 18, 2019

Uh oh!

skonto commented Oct 21, 2019

Uh oh!

skonto commented Oct 23, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dongjoon-hyun commented Oct 24, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dongjoon-hyun Oct 24, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dongjoon-hyun Oct 24, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

skonto Oct 27, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dongjoon-hyun Oct 24, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dongjoon-hyun left a comment

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Oct 24, 2019

Uh oh!

SparkQA commented Oct 24, 2019

Uh oh!

SparkQA commented Oct 24, 2019

Uh oh!

[SPARK-27900][CORE][K8s] Add `spark.driver.killOnOOMError` flag in cluster mode #26161

[SPARK-27900][CORE][K8s] Add `spark.driver.killOnOOMError` flag in cluster mode #26161

skonto commented Oct 18, 2019 •

edited

Loading

skonto commented Oct 23, 2019 •

edited

Loading

dongjoon-hyun Oct 24, 2019 •

edited

Loading

dongjoon-hyun Oct 24, 2019 •

edited

Loading

skonto Oct 27, 2019 •

edited

Loading

dongjoon-hyun Oct 24, 2019 •

edited

Loading

skonto commented Nov 8, 2019 •

edited

Loading

skonto commented Nov 23, 2020 •

edited

Loading