You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by sryza <gi...@git.apache.org> on 2014/10/10 03:31:35 UTC

[GitHub] spark pull request: SPARK-3837. Warn when YARN kills containers fo...

GitHub user sryza opened a pull request:

    https://github.com/apache/spark/pull/2744

    SPARK-3837. Warn when YARN kills containers for exceeding memory limits

    I verified triggered the issue and verified the message gets printed on a pseudo-distributed cluster.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/sryza/spark sandy-spark-3837

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/spark/pull/2744.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #2744
    
----
commit af111851c4c7fb97c84ffd6d90a1fcd1a1065aed
Author: Sandy Ryza <sa...@cloudera.com>
Date:   2014-10-10T01:07:43Z

    SPARK-3837. Warn when YARN kills containers for exceeding memory limits

----


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3837. Warn when YARN kills containers fo...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/spark/pull/2744


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3837. Warn when YARN kills containers fo...

Posted by sryza <gi...@git.apache.org>.
Github user sryza commented on a diff in the pull request:

    https://github.com/apache/spark/pull/2744#discussion_r19580322
  
    --- Diff: yarn/common/src/main/scala/org/apache/spark/deploy/yarn/YarnAllocator.scala ---
    @@ -330,12 +331,21 @@ private[yarn] abstract class YarnAllocator(
               logInfo("Completed container %s (state: %s, exit status: %s)".format(
                 containerId,
                 completedContainer.getState,
    -            completedContainer.getExitStatus()))
    +            completedContainer.getExitStatus))
               // Hadoop 2.2.X added a ContainerExitStatus we should switch to use
               // there are some exit status' we shouldn't necessarily count against us, but for
               // now I think its ok as none of the containers are expected to exit
    -          if (completedContainer.getExitStatus() != 0) {
    -            logInfo("Container marked as failed: " + containerId)
    +          if (completedContainer.getExitStatus == -103) { // vmem limit exceeded
    --- End diff --
    
    The constants don't exist in earlier versions of the 2.x line either


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3837. Warn when YARN kills containers fo...

Posted by sryza <gi...@git.apache.org>.
Github user sryza commented on the pull request:

    https://github.com/apache/spark/pull/2744#issuecomment-61319062
  
    I'll look into it
    
    > On Oct 31, 2014, at 12:36 PM, Shivaram Venkataraman <no...@github.com> wrote:
    > 
    > This patch seems to have broken the build ? See https://amplab.cs.berkeley.edu/jenkins/job/Spark-Master-Maven-with-YARN/HADOOP_PROFILE=hadoop-2.4,label=centos/821/console for an example
    > 
    > —
    > Reply to this email directly or view it on GitHub.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3837. Warn when YARN kills containers fo...

Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/2744#discussion_r19491803
  
    --- Diff: yarn/common/src/test/scala/org/apache/spark/deploy/yarn/YarnAllocatorSuite.scala ---
    @@ -0,0 +1,34 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *    http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.spark.deploy.yarn
    +
    +import org.apache.spark.deploy.yarn.MemLimitLogger._
    +import org.scalatest.FunSuite
    +
    +class YarnAllocatorSuite extends FunSuite {
    +  test("memory exceeded diagnostic regexes") {
    +    val diagnostics =
    +      "Container [pid=12465,containerID=container_1412887393566_0003_01_000002] is running " +
    +      "beyond physical memory limits. Current usage: 2.1 MB of 2 GB physical memory used; " +
    --- End diff --
    
    Is this an actual message from Yarn logs? Because 2.1 MB < 2 GB. :-)


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3837. Warn when YARN kills containers fo...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/2744#issuecomment-58602302
  
      [QA tests have started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/21564/consoleFull) for   PR 2744 at commit [`af11185`](https://github.com/apache/spark/commit/af111851c4c7fb97c84ffd6d90a1fcd1a1065aed).
     * This patch merges cleanly.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3837. Warn when YARN kills containers fo...

Posted by tgravescs <gi...@git.apache.org>.
Github user tgravescs commented on the pull request:

    https://github.com/apache/spark/pull/2744#issuecomment-60758160
  
    Jenkins, test this please.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3837. Warn when YARN kills containers fo...

Posted by jdanbrown <gi...@git.apache.org>.
Github user jdanbrown commented on the pull request:

    https://github.com/apache/spark/pull/2744#issuecomment-62656350
  
    @sryza I came across your PR while searching for ways to debug pyspark yarn containers running over yarn memory limits. This change will be very helpful; what are your thoughts on dumping the container process listing as well, like yarn MR tasks do? That's killer for figuring out which process in your container is out of control, like in the case of pyspark where you have a jvm and python processes to deal with.
    
    Example:
    - https://gist.githubusercontent.com/jdanbrown/cef65e9082b1f32d085e/raw/83347253348f246aa4f9989993e8fb6db14db313/gistfile1.txt


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3837. Warn when YARN kills containers fo...

Posted by sryza <gi...@git.apache.org>.
Github user sryza commented on the pull request:

    https://github.com/apache/spark/pull/2744#issuecomment-62662666
  
    @jdanbrown that seems reasonable.  Mind filing a JIRA for it?


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3837. Warn when YARN kills containers fo...

Posted by shivaram <gi...@git.apache.org>.
Github user shivaram commented on the pull request:

    https://github.com/apache/spark/pull/2744#issuecomment-61317915
  
    This patch seems to have broken the build ? See https://amplab.cs.berkeley.edu/jenkins/job/Spark-Master-Maven-with-YARN/HADOOP_PROFILE=hadoop-2.4,label=centos/821/console for an example


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3837. Warn when YARN kills containers fo...

Posted by sryza <gi...@git.apache.org>.
Github user sryza commented on the pull request:

    https://github.com/apache/spark/pull/2744#issuecomment-61026172
  
    Updated patch incorporates review comments


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3837. Warn when YARN kills containers fo...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/2744#issuecomment-58605740
  
      [QA tests have finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/21564/consoleFull) for   PR 2744 at commit [`af11185`](https://github.com/apache/spark/commit/af111851c4c7fb97c84ffd6d90a1fcd1a1065aed).
     * This patch **passes all tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3837. Warn when YARN kills containers fo...

Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/2744#discussion_r19491256
  
    --- Diff: yarn/common/src/main/scala/org/apache/spark/deploy/yarn/YarnAllocator.scala ---
    @@ -330,12 +331,21 @@ private[yarn] abstract class YarnAllocator(
               logInfo("Completed container %s (state: %s, exit status: %s)".format(
                 containerId,
                 completedContainer.getState,
    -            completedContainer.getExitStatus()))
    +            completedContainer.getExitStatus))
               // Hadoop 2.2.X added a ContainerExitStatus we should switch to use
               // there are some exit status' we shouldn't necessarily count against us, but for
               // now I think its ok as none of the containers are expected to exit
    -          if (completedContainer.getExitStatus() != 0) {
    -            logInfo("Container marked as failed: " + containerId)
    +          if (completedContainer.getExitStatus == -103) { // vmem limit exceeded
    --- End diff --
    
    `ContainerExitStatus` is `@Public`. Does it not exist in yarn-alpha?
    
    If that's why you're using the numbers directly, I'd leave a comment saying this can be cleaned up when yarn-alpha is removed.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3837. Warn when YARN kills containers fo...

Posted by shivaram <gi...@git.apache.org>.
Github user shivaram commented on the pull request:

    https://github.com/apache/spark/pull/2744#issuecomment-61319258
  
    I have a commit to just comment out the test at https://github.com/shivaram/spark-1/compare/fix-yarn-build?expand=1 -- But if you have a better fix, we can use that.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3837. Warn when YARN kills containers fo...

Posted by tgravescs <gi...@git.apache.org>.
Github user tgravescs commented on the pull request:

    https://github.com/apache/spark/pull/2744#issuecomment-61261231
  
    looks good. thanks @sryza!


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3837. Warn when YARN kills containers fo...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/2744#issuecomment-58602139
  
    Can one of the admins verify this patch?


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3837. Warn when YARN kills containers fo...

Posted by tgravescs <gi...@git.apache.org>.
Github user tgravescs commented on a diff in the pull request:

    https://github.com/apache/spark/pull/2744#discussion_r19472179
  
    --- Diff: yarn/common/src/main/scala/org/apache/spark/deploy/yarn/YarnAllocator.scala ---
    @@ -330,12 +331,21 @@ private[yarn] abstract class YarnAllocator(
               logInfo("Completed container %s (state: %s, exit status: %s)".format(
                 containerId,
                 completedContainer.getState,
    -            completedContainer.getExitStatus()))
    +            completedContainer.getExitStatus))
               // Hadoop 2.2.X added a ContainerExitStatus we should switch to use
               // there are some exit status' we shouldn't necessarily count against us, but for
               // now I think its ok as none of the containers are expected to exit
    -          if (completedContainer.getExitStatus() != 0) {
    -            logInfo("Container marked as failed: " + containerId)
    +          if (completedContainer.getExitStatus == -103) { // vmem limit exceeded
    +            logWarning(MemLimitLogger.memLimitExceededLogMessage(
    +              completedContainer.getDiagnostics,
    +              MemLimitLogger.VMEM_EXCEEDED_PATTERN))
    +          } else if (completedContainer.getExitStatus == -104) { // pmem limit exceeded
    +            logWarning(MemLimitLogger.memLimitExceededLogMessage(
    +              completedContainer.getDiagnostics,
    +              MemLimitLogger.PMEM_EXCEEDED_PATTERN))
    +          } else if (completedContainer.getExitStatus != 0) {
    +            logInfo("Container marked as failed: " + containerId + ". Exit status: " +
    --- End diff --
    
    Any reason we don't print the diagnostics message here also?


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3837. Warn when YARN kills containers fo...

Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/2744#discussion_r19492068
  
    --- Diff: yarn/common/src/main/scala/org/apache/spark/deploy/yarn/YarnAllocator.scala ---
    @@ -463,3 +473,19 @@ private[yarn] abstract class YarnAllocator(
       }
     
     }
    +
    +private[yarn] object MemLimitLogger {
    --- End diff --
    
    Mostly personal preference I guess, but it feels too heavy-handed to add an object for this (instead of placing these things inside YarnAllocator directly ).


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request: SPARK-3837. Warn when YARN kills containers fo...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/2744#issuecomment-58605744
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/21564/Test PASSed.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org