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/08/16 10:02:39 UTC
[GitHub] spark pull request: SPARK-3082. yarn.Client.logClusterResourceDeta...
GitHub user sryza opened a pull request:
https://github.com/apache/spark/pull/1984
SPARK-3082. yarn.Client.logClusterResourceDetails throws NPE if requeste...
...d queue doesn't exist
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/sryza/spark sandy-spark-3082
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/1984.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 #1984
----
commit e33a35e116067a65526ab39d5be1093e22ad7caa
Author: Sandy Ryza <sa...@cloudera.com>
Date: 2014-08-16T07:54:25Z
SPARK-3082. yarn.Client.logClusterResourceDetails throws NPE if requested queue doesn't exist
----
---
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-3082. yarn.Client.logClusterResourceDeta...
Posted by sryza <gi...@git.apache.org>.
Github user sryza commented on a diff in the pull request:
https://github.com/apache/spark/pull/1984#discussion_r16372445
--- Diff: yarn/stable/src/main/scala/org/apache/spark/deploy/yarn/Client.scala ---
@@ -103,13 +103,17 @@ class Client(clientArgs: ClientArguments, hadoopConf: Configuration, spConf: Spa
clusterMetrics.getNumNodeManagers)
val queueInfo: QueueInfo = yarnClient.getQueueInfo(args.amQueue)
--- End diff --
What makes you think that? amQueue gets set with:
> var amQueue = sparkConf.get("QUEUE", "default")
So I don't think it can end up null.
---
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-3082. yarn.Client.logClusterResourceDeta...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/1984#issuecomment-52387825
[QA tests have finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/18672/consoleFull) for PR 1984 at commit [`e33a35e`](https://github.com/apache/spark/commit/e33a35e116067a65526ab39d5be1093e22ad7caa).
* This patch **fails** unit tests.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `abstract class Serializer `
* `abstract class SerializerInstance `
* `abstract class SerializationStream `
* `abstract class DeserializationStream `
* `class ShuffleBlockManager(blockManager: BlockManager,`
---
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-3082. yarn.Client.logClusterResourceDeta...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on a diff in the pull request:
https://github.com/apache/spark/pull/1984#discussion_r16372829
--- Diff: yarn/stable/src/main/scala/org/apache/spark/deploy/yarn/Client.scala ---
@@ -103,13 +103,17 @@ class Client(clientArgs: ClientArguments, hadoopConf: Configuration, spConf: Spa
clusterMetrics.getNumNodeManagers)
val queueInfo: QueueInfo = yarnClient.getQueueInfo(args.amQueue)
- logInfo( """Queue info ... queueName: %s, queueCurrentCapacity: %s, queueMaxCapacity: %s,
+ if (queueInfo != null) {
+ logInfo( """Queue info ... queueName: %s, queueCurrentCapacity: %s, queueMaxCapacity: %s,
queueApplicationCount = %s, queueChildQueueCount = %s""".format(
queueInfo.getQueueName,
queueInfo.getCurrentCapacity,
queueInfo.getMaximumCapacity,
queueInfo.getApplications.size,
queueInfo.getChildQueues.size))
+ } else {
+ logInfo("Queue %s not found.".format(args.amQueue))
--- End diff --
I guess this question is more "what do other apps do?" than anything else. But it sounds like the requested queue not existing would be reason for a more explicit failure?
In the very least I'd promote this to logWarning(), but I'm actually thinking just failing the app, since the requested queue not existing might lead to unwanted behavior.
---
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-3082. yarn.Client.logClusterResourceDeta...
Posted by sryza <gi...@git.apache.org>.
Github user sryza commented on a diff in the pull request:
https://github.com/apache/spark/pull/1984#discussion_r16378515
--- Diff: yarn/stable/src/main/scala/org/apache/spark/deploy/yarn/Client.scala ---
@@ -103,13 +103,17 @@ class Client(clientArgs: ClientArguments, hadoopConf: Configuration, spConf: Spa
clusterMetrics.getNumNodeManagers)
val queueInfo: QueueInfo = yarnClient.getQueueInfo(args.amQueue)
- logInfo( """Queue info ... queueName: %s, queueCurrentCapacity: %s, queueMaxCapacity: %s,
+ if (queueInfo != null) {
+ logInfo( """Queue info ... queueName: %s, queueCurrentCapacity: %s, queueMaxCapacity: %s,
queueApplicationCount = %s, queueChildQueueCount = %s""".format(
queueInfo.getQueueName,
queueInfo.getCurrentCapacity,
queueInfo.getMaximumCapacity,
queueInfo.getApplications.size,
queueInfo.getChildQueues.size))
+ } else {
+ logInfo("Queue %s not found.".format(args.amQueue))
--- End diff --
It's not uncommon for YARN to place an app into a queue different than the one requested. The Fair Scheduler has policies for this and this Capacity Scheduler is about to add them. So I don't think failing the job is the right move. On further thought, it actually might make sense to avoid reporting this queue info at all until we can get it from the queue that the app actually ends up in.
---
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-3082. yarn.Client.logClusterResourceDeta...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/1984#discussion_r16370191
--- Diff: yarn/stable/src/main/scala/org/apache/spark/deploy/yarn/Client.scala ---
@@ -103,13 +103,17 @@ class Client(clientArgs: ClientArguments, hadoopConf: Configuration, spConf: Spa
clusterMetrics.getNumNodeManagers)
val queueInfo: QueueInfo = yarnClient.getQueueInfo(args.amQueue)
--- End diff --
It seems that the culprit is that `args.amQueue` is null. Should we do a null check on that instead?
---
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-3082. yarn.Client.logClusterResourceDeta...
Posted by sryza <gi...@git.apache.org>.
Github user sryza commented on the pull request:
https://github.com/apache/spark/pull/1984#issuecomment-52552878
Posted a patch that removes the queue resources log message entirely
---
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-3082. yarn.Client.logClusterResourceDeta...
Posted by sryza <gi...@git.apache.org>.
Github user sryza commented on a diff in the pull request:
https://github.com/apache/spark/pull/1984#discussion_r16380717
--- Diff: yarn/stable/src/main/scala/org/apache/spark/deploy/yarn/Client.scala ---
@@ -103,13 +103,17 @@ class Client(clientArgs: ClientArguments, hadoopConf: Configuration, spConf: Spa
clusterMetrics.getNumNodeManagers)
val queueInfo: QueueInfo = yarnClient.getQueueInfo(args.amQueue)
- logInfo( """Queue info ... queueName: %s, queueCurrentCapacity: %s, queueMaxCapacity: %s,
+ if (queueInfo != null) {
+ logInfo( """Queue info ... queueName: %s, queueCurrentCapacity: %s, queueMaxCapacity: %s,
queueApplicationCount = %s, queueChildQueueCount = %s""".format(
queueInfo.getQueueName,
queueInfo.getCurrentCapacity,
queueInfo.getMaximumCapacity,
queueInfo.getApplications.size,
queueInfo.getChildQueues.size))
+ } else {
+ logInfo("Queue %s not found.".format(args.amQueue))
--- End diff --
I mean the fair scheduler placement policies. Placement policies can be configured to ignore the queue passed in (or to only ignore it if "default" is specified).
---
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-3082. yarn.Client.logClusterResourceDeta...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/1984
---
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-3082. yarn.Client.logClusterResourceDeta...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/1984#issuecomment-52387014
[QA tests have started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/18672/consoleFull) for PR 1984 at commit [`e33a35e`](https://github.com/apache/spark/commit/e33a35e116067a65526ab39d5be1093e22ad7caa).
* 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-3082. yarn.Client.logClusterResourceDeta...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/1984#issuecomment-52530393
Hi @sryza, thanks for fixing this. I am actually somewhat confused by this because this code has apparently been around for a while, and we have not seen this issue until recently. Do you know what has been added recently that might have caused this to surface?
Also it would be good to fix the equivalent in `org/apache/spark/yarn/alpha/` as well.
---
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-3082. yarn.Client.logClusterResourceDeta...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/1984#issuecomment-52553694
[QA tests have started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/18781/consoleFull) for PR 1984 at commit [`fe08c37`](https://github.com/apache/spark/commit/fe08c3743f749c43fb95631f3fba7df27304ee78).
* 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-3082. yarn.Client.logClusterResourceDeta...
Posted by tgravescs <gi...@git.apache.org>.
Github user tgravescs commented on a diff in the pull request:
https://github.com/apache/spark/pull/1984#discussion_r16373929
--- Diff: yarn/stable/src/main/scala/org/apache/spark/deploy/yarn/Client.scala ---
@@ -103,13 +103,17 @@ class Client(clientArgs: ClientArguments, hadoopConf: Configuration, spConf: Spa
clusterMetrics.getNumNodeManagers)
val queueInfo: QueueInfo = yarnClient.getQueueInfo(args.amQueue)
- logInfo( """Queue info ... queueName: %s, queueCurrentCapacity: %s, queueMaxCapacity: %s,
+ if (queueInfo != null) {
+ logInfo( """Queue info ... queueName: %s, queueCurrentCapacity: %s, queueMaxCapacity: %s,
queueApplicationCount = %s, queueChildQueueCount = %s""".format(
queueInfo.getQueueName,
queueInfo.getCurrentCapacity,
queueInfo.getMaximumCapacity,
queueInfo.getApplications.size,
queueInfo.getChildQueues.size))
+ } else {
+ logInfo("Queue %s not found.".format(args.amQueue))
--- End diff --
I have also run into this before and the message isn't very user friendly as to why the job failed. The job isn't going to run so I think logError and throwing an exception and exiting would be better. It would actually be nice to move this up to validateArgs but I don't want to have to look it up twice 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-3082. yarn.Client.logClusterResourceDeta...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/1984#issuecomment-53819790
Thanks, I've merged this into master and 1.1.
---
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-3082. yarn.Client.logClusterResourceDeta...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/1984#issuecomment-52560207
[QA tests have finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/18781/consoleFull) for PR 1984 at commit [`fe08c37`](https://github.com/apache/spark/commit/fe08c3743f749c43fb95631f3fba7df27304ee78).
* This patch **passes** unit 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-3082. yarn.Client.logClusterResourceDeta...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/1984#discussion_r16370332
--- Diff: yarn/stable/src/main/scala/org/apache/spark/deploy/yarn/Client.scala ---
@@ -103,13 +103,17 @@ class Client(clientArgs: ClientArguments, hadoopConf: Configuration, spConf: Spa
clusterMetrics.getNumNodeManagers)
val queueInfo: QueueInfo = yarnClient.getQueueInfo(args.amQueue)
--- End diff --
Also, is it safe to call `appContext.setQueue(null)` in L81? (and setApplicationName)
---
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-3082. yarn.Client.logClusterResourceDeta...
Posted by tgravescs <gi...@git.apache.org>.
Github user tgravescs commented on the pull request:
https://github.com/apache/spark/pull/1984#issuecomment-52639767
The changes lgtm.
---
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-3082. yarn.Client.logClusterResourceDeta...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/1984#discussion_r16379892
--- Diff: yarn/stable/src/main/scala/org/apache/spark/deploy/yarn/Client.scala ---
@@ -103,13 +103,17 @@ class Client(clientArgs: ClientArguments, hadoopConf: Configuration, spConf: Spa
clusterMetrics.getNumNodeManagers)
val queueInfo: QueueInfo = yarnClient.getQueueInfo(args.amQueue)
--- End diff --
Oh I see. Many of the other arguments in `ClientArguments` were initialized to null so I thought this was the same. Then it's fine.
(On an unrelated note initializing the queue to this value makes little sense to me. We don't set the `QUEUE` attribute anywhere in the code, and creating a fresh `SparkConf` won't actually pick this config up because it is not prefixed with "spark". This seems to be always initialized to "default")
---
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-3082. yarn.Client.logClusterResourceDeta...
Posted by tgravescs <gi...@git.apache.org>.
Github user tgravescs commented on a diff in the pull request:
https://github.com/apache/spark/pull/1984#discussion_r16380174
--- Diff: yarn/stable/src/main/scala/org/apache/spark/deploy/yarn/Client.scala ---
@@ -103,13 +103,17 @@ class Client(clientArgs: ClientArguments, hadoopConf: Configuration, spConf: Spa
clusterMetrics.getNumNodeManagers)
val queueInfo: QueueInfo = yarnClient.getQueueInfo(args.amQueue)
- logInfo( """Queue info ... queueName: %s, queueCurrentCapacity: %s, queueMaxCapacity: %s,
+ if (queueInfo != null) {
+ logInfo( """Queue info ... queueName: %s, queueCurrentCapacity: %s, queueMaxCapacity: %s,
queueApplicationCount = %s, queueChildQueueCount = %s""".format(
queueInfo.getQueueName,
queueInfo.getCurrentCapacity,
queueInfo.getMaximumCapacity,
queueInfo.getApplications.size,
queueInfo.getChildQueues.size))
+ } else {
+ logInfo("Queue %s not found.".format(args.amQueue))
--- End diff --
Are you referring to the functionality to move application between queues or the fair scheduler placement policy?
in the placement policy does it basically ignore the queue passed in then?
---
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