You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by wangyum <gi...@git.apache.org> on 2018/11/14 08:07:09 UTC
[GitHub] spark pull request #23030: [MINOR][YARN] Make memLimitExceededLogMessage mor...
GitHub user wangyum opened a pull request:
https://github.com/apache/spark/pull/23030
[MINOR][YARN] Make memLimitExceededLogMessage more clean
## What changes were proposed in this pull request?
Current `memLimitExceededLogMessage`:
<img src="https://user-images.githubusercontent.com/5399861/48467789-ec8e1000-e824-11e8-91fc-280d342e1bf3.png" width="360">
It‘s not very clear, because physical memory exceeds but suggestion contains virtual memory config. This pr makes it more clear and replace deprecated config: ```spark.yarn.executor.memoryOverhead```.
## How was this patch tested?
manual tests
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/wangyum/spark EXECUTOR_MEMORY_OVERHEAD
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/23030.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 #23030
----
commit e7ff43d0056e38033ae2a2edf81b296797bb46e0
Author: Yuming Wang <yu...@...>
Date: 2018-11-14T07:46:58Z
Make memLimitExceededLogMessage more clean
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23030: [MINOR][YARN] Make memLimitExceededLogMessage more clean
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23030
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/5096/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23030: [MINOR][YARN] Make memLimitExceededLogMessage more clean
Posted by wangyum <gi...@git.apache.org>.
Github user wangyum commented on the issue:
https://github.com/apache/spark/pull/23030
cc @vanzin
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23030: [MINOR][YARN] Make memLimitExceededLogMessage more clean
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/23030
**[Test build #98877 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98877/testReport)** for PR 23030 at commit [`965a984`](https://github.com/apache/spark/commit/965a9840f0e1a97945014c19b6bb281afdfee599).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23030: [MINOR][YARN] Make memLimitExceededLogMessage more clean
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23030
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23030: [MINOR][YARN] Make memLimitExceededLogMessage more clean
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23030
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/5080/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23030: [MINOR][YARN] Make memLimitExceededLogMessage more clean
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/23030
**[Test build #98946 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98946/testReport)** for PR 23030 at commit [`ca008cd`](https://github.com/apache/spark/commit/ca008cd6bb16a4b5c3156b572bb4f05034230912).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23030: [MINOR][YARN] Make memLimitExceededLogMessage more clean
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/23030
**[Test build #98944 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98944/testReport)** for PR 23030 at commit [`9ad6df4`](https://github.com/apache/spark/commit/9ad6df4ce35c0be9bf69ccfaa1b9729f480a12d7).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23030: [MINOR][YARN] Make memLimitExceededLogMessage more clean
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/23030
**[Test build #98910 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98910/testReport)** for PR 23030 at commit [`8ab6c3b`](https://github.com/apache/spark/commit/8ab6c3bce2a88ec0efed9bc012e21d1cc997eb46).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23030: [MINOR][YARN] Make memLimitExceededLogMessage more clean
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/23030
Merged to master
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23030: [MINOR][YARN] Make memLimitExceededLogMessage more clean
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23030
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/98814/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #23030: [MINOR][YARN] Make memLimitExceededLogMessage mor...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:
https://github.com/apache/spark/pull/23030#discussion_r234304621
--- Diff: resource-managers/yarn/src/main/scala/org/apache/spark/deploy/yarn/YarnAllocator.scala ---
@@ -598,13 +597,25 @@ private[yarn] class YarnAllocator(
(false, s"Container ${containerId}${onHostStr} was preempted.")
// Should probably still count memory exceeded exit codes towards task failures
case VMEM_EXCEEDED_EXIT_CODE =>
- (true, memLimitExceededLogMessage(
- completedContainer.getDiagnostics,
- VMEM_EXCEEDED_PATTERN))
+ val vmemExceededPattern = raw"$MEM_REGEX of $MEM_REGEX virtual memory used".r
+ val diag = vmemExceededPattern.findFirstIn(completedContainer.getDiagnostics)
+ .map(_.concat(".")).getOrElse("")
+ val additional = if (conf.getBoolean(YarnConfiguration.NM_VMEM_CHECK_ENABLED,
--- End diff --
You didn't answer my question. How can you even get here if this config is disabled?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23030: [MINOR][YARN] Make memLimitExceededLogMessage more clean
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23030
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/98916/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23030: [MINOR][YARN] Make memLimitExceededLogMessage more clean
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23030
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23030: [MINOR][YARN] Make memLimitExceededLogMessage more clean
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23030
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23030: [MINOR][YARN] Make memLimitExceededLogMessage more clean
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23030
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/5015/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23030: [MINOR][YARN] Make memLimitExceededLogMessage more clean
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23030
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23030: [MINOR][YARN] Make memLimitExceededLogMessage more clean
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23030
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/5043/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23030: [MINOR][YARN] Make memLimitExceededLogMessage more clean
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23030
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/98859/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #23030: [MINOR][YARN] Make memLimitExceededLogMessage mor...
Posted by wangyum <gi...@git.apache.org>.
Github user wangyum commented on a diff in the pull request:
https://github.com/apache/spark/pull/23030#discussion_r234386679
--- Diff: resource-managers/yarn/src/main/scala/org/apache/spark/deploy/yarn/YarnAllocator.scala ---
@@ -598,13 +597,25 @@ private[yarn] class YarnAllocator(
(false, s"Container ${containerId}${onHostStr} was preempted.")
// Should probably still count memory exceeded exit codes towards task failures
case VMEM_EXCEEDED_EXIT_CODE =>
- (true, memLimitExceededLogMessage(
- completedContainer.getDiagnostics,
- VMEM_EXCEEDED_PATTERN))
+ val vmemExceededPattern = raw"$MEM_REGEX of $MEM_REGEX virtual memory used".r
+ val diag = vmemExceededPattern.findFirstIn(completedContainer.getDiagnostics)
+ .map(_.concat(".")).getOrElse("")
+ val additional = if (conf.getBoolean(YarnConfiguration.NM_VMEM_CHECK_ENABLED,
--- End diff --
I see.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23030: [MINOR][YARN] Make memLimitExceededLogMessage more clean
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23030
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/5056/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23030: [MINOR][YARN] Make memLimitExceededLogMessage more clean
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23030
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #23030: [MINOR][YARN] Make memLimitExceededLogMessage mor...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:
https://github.com/apache/spark/pull/23030#discussion_r234390445
--- Diff: resource-managers/yarn/src/main/scala/org/apache/spark/deploy/yarn/YarnAllocator.scala ---
@@ -598,13 +597,20 @@ private[yarn] class YarnAllocator(
(false, s"Container ${containerId}${onHostStr} was preempted.")
// Should probably still count memory exceeded exit codes towards task failures
case VMEM_EXCEEDED_EXIT_CODE =>
- (true, memLimitExceededLogMessage(
- completedContainer.getDiagnostics,
- VMEM_EXCEEDED_PATTERN))
+ val vmemExceededPattern = raw"$MEM_REGEX of $MEM_REGEX virtual memory used".r
+ val diag = vmemExceededPattern.findFirstIn(completedContainer.getDiagnostics)
+ .map(_.concat(".")).getOrElse("")
+ val message = "Container killed by YARN for exceeding virtual memory limits. " +
+ s"$diag Consider boosting ${EXECUTOR_MEMORY_OVERHEAD.key} or disabling " +
+ s"${YarnConfiguration.NM_VMEM_CHECK_ENABLED} because of YARN-4714."
--- End diff --
Now you removed the other config option I asked you to add... :-/
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23030: [MINOR][YARN] Make memLimitExceededLogMessage more clean
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/23030
Because there are just two calls to this method, and now they behave pretty differently based on one argument, how about just inlining both calls and the regexes to make this simpler?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #23030: [MINOR][YARN] Make memLimitExceededLogMessage mor...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/23030#discussion_r234232203
--- Diff: resource-managers/yarn/src/main/scala/org/apache/spark/deploy/yarn/YarnAllocator.scala ---
@@ -735,18 +744,8 @@ private[yarn] class YarnAllocator(
private object YarnAllocator {
val MEM_REGEX = "[0-9.]+ [KMG]B"
- val PMEM_EXCEEDED_PATTERN =
- Pattern.compile(s"$MEM_REGEX of $MEM_REGEX physical memory used")
- val VMEM_EXCEEDED_PATTERN =
- Pattern.compile(s"$MEM_REGEX of $MEM_REGEX virtual memory used")
+ val PMEM_EXCEEDED_PATTERN = raw"$MEM_REGEX of $MEM_REGEX physical memory used".r
--- End diff --
We can still inline these patterns right?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23030: [MINOR][YARN] Make memLimitExceededLogMessage more clean
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23030
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23030: [MINOR][YARN] Make memLimitExceededLogMessage more clean
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23030
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/98877/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23030: [MINOR][YARN] Make memLimitExceededLogMessage more clean
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23030
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #23030: [MINOR][YARN] Make memLimitExceededLogMessage mor...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/23030#discussion_r233887452
--- Diff: resource-managers/yarn/src/test/scala/org/apache/spark/deploy/yarn/YarnAllocatorSuite.scala ---
@@ -378,14 +380,16 @@ class YarnAllocatorSuite extends SparkFunSuite with Matchers with BeforeAndAfter
}
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; " +
- "5.8 GB of 4.2 GB virtual memory used. Killing container."
- val vmemMsg = memLimitExceededLogMessage(diagnostics, VMEM_EXCEEDED_PATTERN)
- val pmemMsg = memLimitExceededLogMessage(diagnostics, PMEM_EXCEEDED_PATTERN)
- assert(vmemMsg.contains("5.8 GB of 4.2 GB virtual memory used."))
- assert(pmemMsg.contains("2.1 MB of 2 GB physical memory used."))
+ def logMessage(pattern: Pattern): String = {
--- End diff --
I don't feel strongly about it, but I could just remove this test too, and inline the regexes to simplify this. This was just testing the exact error message and I don't know how useful that is.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #23030: [MINOR][YARN] Make memLimitExceededLogMessage mor...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:
https://github.com/apache/spark/pull/23030#discussion_r233948762
--- Diff: resource-managers/yarn/src/main/scala/org/apache/spark/deploy/yarn/YarnAllocator.scala ---
@@ -598,13 +598,24 @@ private[yarn] class YarnAllocator(
(false, s"Container ${containerId}${onHostStr} was preempted.")
// Should probably still count memory exceeded exit codes towards task failures
case VMEM_EXCEEDED_EXIT_CODE =>
- (true, memLimitExceededLogMessage(
- completedContainer.getDiagnostics,
- VMEM_EXCEEDED_PATTERN))
+ val suggestion = if (conf.getBoolean(YarnConfiguration.NM_VMEM_CHECK_ENABLED,
--- End diff --
Ah, and another option that can help here is `yarn.nodemanager.vmem-pmem-ratio`.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23030: [MINOR][YARN] Make memLimitExceededLogMessage more clean
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23030
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/98910/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23030: [MINOR][YARN] Make memLimitExceededLogMessage more clean
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23030
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23030: [MINOR][YARN] Make memLimitExceededLogMessage more clean
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23030
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/98944/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23030: [MINOR][YARN] Make memLimitExceededLogMessage more clean
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23030
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23030: [MINOR][YARN] Make memLimitExceededLogMessage more clean
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/23030
**[Test build #98859 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98859/testReport)** for PR 23030 at commit [`44767f8`](https://github.com/apache/spark/commit/44767f85668b87ad0f00af6e103b845067ea8e2c).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23030: [MINOR][YARN] Make memLimitExceededLogMessage more clean
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/23030
**[Test build #98946 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98946/testReport)** for PR 23030 at commit [`ca008cd`](https://github.com/apache/spark/commit/ca008cd6bb16a4b5c3156b572bb4f05034230912).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23030: [MINOR][YARN] Make memLimitExceededLogMessage more clean
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23030
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #23030: [MINOR][YARN] Make memLimitExceededLogMessage mor...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:
https://github.com/apache/spark/pull/23030#discussion_r233946890
--- Diff: resource-managers/yarn/src/main/scala/org/apache/spark/deploy/yarn/YarnAllocator.scala ---
@@ -598,13 +598,24 @@ private[yarn] class YarnAllocator(
(false, s"Container ${containerId}${onHostStr} was preempted.")
// Should probably still count memory exceeded exit codes towards task failures
case VMEM_EXCEEDED_EXIT_CODE =>
- (true, memLimitExceededLogMessage(
- completedContainer.getDiagnostics,
- VMEM_EXCEEDED_PATTERN))
+ val suggestion = if (conf.getBoolean(YarnConfiguration.NM_VMEM_CHECK_ENABLED,
+ YarnConfiguration.DEFAULT_NM_VMEM_CHECK_ENABLED)) {
+ s"Consider disabling ${YarnConfiguration.NM_VMEM_CHECK_ENABLED} because of YARN-4714"
+ } else {
+ ""
+ }
+ val matcher = VMEM_EXCEEDED_PATTERN.matcher(completedContainer.getDiagnostics)
+ val diag = if (matcher.find()) " " + matcher.group() + "." else ""
+ val message =
+ s"Container killed by YARN for exceeding virtual memory limits.$diag $suggestion."
+ (true, message)
case PMEM_EXCEEDED_EXIT_CODE =>
- (true, memLimitExceededLogMessage(
- completedContainer.getDiagnostics,
- PMEM_EXCEEDED_PATTERN))
+ val suggestion = s"Consider boosting ${EXECUTOR_MEMORY_OVERHEAD.key}"
--- End diff --
Why isn't this a suggestion in the vmem case too? It can help, even if it's a sledgehammer. Otherwise, you're basically telling the user in that case: "whatcha gonna do? it failed."
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23030: [MINOR][YARN] Make memLimitExceededLogMessage more clean
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/23030
**[Test build #98910 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98910/testReport)** for PR 23030 at commit [`8ab6c3b`](https://github.com/apache/spark/commit/8ab6c3bce2a88ec0efed9bc012e21d1cc997eb46).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23030: [MINOR][YARN] Make memLimitExceededLogMessage more clean
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23030
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/98946/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #23030: [MINOR][YARN] Make memLimitExceededLogMessage mor...
Posted by wangyum <gi...@git.apache.org>.
Github user wangyum commented on a diff in the pull request:
https://github.com/apache/spark/pull/23030#discussion_r234233444
--- Diff: resource-managers/yarn/src/main/scala/org/apache/spark/deploy/yarn/YarnAllocator.scala ---
@@ -735,18 +744,8 @@ private[yarn] class YarnAllocator(
private object YarnAllocator {
val MEM_REGEX = "[0-9.]+ [KMG]B"
- val PMEM_EXCEEDED_PATTERN =
- Pattern.compile(s"$MEM_REGEX of $MEM_REGEX physical memory used")
- val VMEM_EXCEEDED_PATTERN =
- Pattern.compile(s"$MEM_REGEX of $MEM_REGEX virtual memory used")
+ val PMEM_EXCEEDED_PATTERN = raw"$MEM_REGEX of $MEM_REGEX physical memory used".r
--- End diff --
Yes. I will do it.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23030: [MINOR][YARN] Make memLimitExceededLogMessage more clean
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23030
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23030: [MINOR][YARN] Make memLimitExceededLogMessage more clean
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23030
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/5098/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #23030: [MINOR][YARN] Make memLimitExceededLogMessage mor...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:
https://github.com/apache/spark/pull/23030#discussion_r233948023
--- Diff: resource-managers/yarn/src/main/scala/org/apache/spark/deploy/yarn/YarnAllocator.scala ---
@@ -598,13 +598,24 @@ private[yarn] class YarnAllocator(
(false, s"Container ${containerId}${onHostStr} was preempted.")
// Should probably still count memory exceeded exit codes towards task failures
case VMEM_EXCEEDED_EXIT_CODE =>
- (true, memLimitExceededLogMessage(
- completedContainer.getDiagnostics,
- VMEM_EXCEEDED_PATTERN))
+ val suggestion = if (conf.getBoolean(YarnConfiguration.NM_VMEM_CHECK_ENABLED,
+ YarnConfiguration.DEFAULT_NM_VMEM_CHECK_ENABLED)) {
+ s"Consider disabling ${YarnConfiguration.NM_VMEM_CHECK_ENABLED} because of YARN-4714"
+ } else {
+ ""
+ }
+ val matcher = VMEM_EXCEEDED_PATTERN.matcher(completedContainer.getDiagnostics)
+ val diag = if (matcher.find()) " " + matcher.group() + "." else ""
+ val message =
+ s"Container killed by YARN for exceeding virtual memory limits.$diag $suggestion."
+ (true, message)
case PMEM_EXCEEDED_EXIT_CODE =>
- (true, memLimitExceededLogMessage(
- completedContainer.getDiagnostics,
- PMEM_EXCEEDED_PATTERN))
+ val suggestion = s"Consider boosting ${EXECUTOR_MEMORY_OVERHEAD.key}"
+ val matcher = PMEM_EXCEEDED_PATTERN.matcher(completedContainer.getDiagnostics)
+ val diag = if (matcher.find()) " " + matcher.group() + "." else ""
+ val message =
+ s"Container killed by YARN for exceeding physical memory limits.$diag $suggestion."
--- End diff --
Ignoring the above comment, `suggestion` is always set, so it could be inlined here.
Separately, it's always sub-optimal when formatting is scattered among different variables (e.g. the period at the end of the suggestion is here, the space before the diag is in the diag message itself).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23030: [MINOR][YARN] Make memLimitExceededLogMessage more clean
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/23030
**[Test build #98916 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98916/testReport)** for PR 23030 at commit [`66b3e7f`](https://github.com/apache/spark/commit/66b3e7f600acb4b630a1387d4e17c73fb11ffce5).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #23030: [MINOR][YARN] Make memLimitExceededLogMessage mor...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/23030
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23030: [MINOR][YARN] Make memLimitExceededLogMessage more clean
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/23030
**[Test build #98814 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98814/testReport)** for PR 23030 at commit [`e7ff43d`](https://github.com/apache/spark/commit/e7ff43d0056e38033ae2a2edf81b296797bb46e0).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23030: [MINOR][YARN] Make memLimitExceededLogMessage more clean
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23030
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/5077/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23030: [MINOR][YARN] Make memLimitExceededLogMessage more clean
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/23030
**[Test build #98916 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98916/testReport)** for PR 23030 at commit [`66b3e7f`](https://github.com/apache/spark/commit/66b3e7f600acb4b630a1387d4e17c73fb11ffce5).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23030: [MINOR][YARN] Make memLimitExceededLogMessage more clean
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/23030
**[Test build #98859 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98859/testReport)** for PR 23030 at commit [`44767f8`](https://github.com/apache/spark/commit/44767f85668b87ad0f00af6e103b845067ea8e2c).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23030: [MINOR][YARN] Make memLimitExceededLogMessage more clean
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23030
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23030: [MINOR][YARN] Make memLimitExceededLogMessage more clean
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23030
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #23030: [MINOR][YARN] Make memLimitExceededLogMessage mor...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/23030#discussion_r233901330
--- Diff: resource-managers/yarn/src/main/scala/org/apache/spark/deploy/yarn/YarnAllocator.scala ---
@@ -598,13 +598,24 @@ private[yarn] class YarnAllocator(
(false, s"Container ${containerId}${onHostStr} was preempted.")
// Should probably still count memory exceeded exit codes towards task failures
case VMEM_EXCEEDED_EXIT_CODE =>
- (true, memLimitExceededLogMessage(
- completedContainer.getDiagnostics,
- VMEM_EXCEEDED_PATTERN))
+ val suggestion = if (conf.getBoolean(YarnConfiguration.NM_VMEM_CHECK_ENABLED,
+ YarnConfiguration.DEFAULT_NM_VMEM_CHECK_ENABLED)) {
+ s"Consider disabling ${YarnConfiguration.NM_VMEM_CHECK_ENABLED} because of YARN-4714"
+ } else {
+ ""
+ }
+ val matcher = VMEM_EXCEEDED_PATTERN.matcher(completedContainer.getDiagnostics)
--- End diff --
You can inline these patterns to further simplify.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23030: [MINOR][YARN] Make memLimitExceededLogMessage more clean
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/23030
**[Test build #98877 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98877/testReport)** for PR 23030 at commit [`965a984`](https://github.com/apache/spark/commit/965a9840f0e1a97945014c19b6bb281afdfee599).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23030: [MINOR][YARN] Make memLimitExceededLogMessage more clean
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/23030
**[Test build #98814 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98814/testReport)** for PR 23030 at commit [`e7ff43d`](https://github.com/apache/spark/commit/e7ff43d0056e38033ae2a2edf81b296797bb46e0).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23030: [MINOR][YARN] Make memLimitExceededLogMessage more clean
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23030
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23030: [MINOR][YARN] Make memLimitExceededLogMessage more clean
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/23030
**[Test build #98944 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/98944/testReport)** for PR 23030 at commit [`9ad6df4`](https://github.com/apache/spark/commit/9ad6df4ce35c0be9bf69ccfaa1b9729f480a12d7).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org