Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[SPARK-6975][Yarn] Fix argument validation error #5551

Closed
wants to merge 2 commits into from
Closed

[SPARK-6975][Yarn] Fix argument validation error #5551

wants to merge 2 commits into from

Conversation

jerryshao
Copy link
Contributor

numExecutors checking is failed when dynamic allocation is enabled with default configuration. Details can be seen is SPARK-6975. @sryza, please help me to review this, not sure is this the correct way, I think previous you change this part :)

srowen
srowen reviewed Apr 17, 2015
View reviewed changes
yarn/src/main/scala/org/apache/spark/deploy/yarn/ClientArguments.scala
"You must specify at least 1 executor!\n" + getUsageMessage())
s"""
|Number of executors $numExecutors is not legal.
|If dynamic allocation is enable, number of executors should at least be 0.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

enabled -> enabled. I think this is simpler to state as "Number of executors was $numExecutors, but must be at least 1 (or 0 if dynamic executor allocation is enabled)."

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I will change the statement.

@SparkQA
Copy link

SparkQA commented Apr 17, 2015

Test build #30466 has finished for PR 5551 at commit 77bdcbd.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.
  • This patch does not change any dependencies.

@SparkQA
Copy link

SparkQA commented Apr 17, 2015

Test build #30471 has finished for PR 5551 at commit 4335da1.

  • This patch fails PySpark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.
  • This patch does not change any dependencies.

@srowen
Copy link
Member

srowen commented Apr 17, 2015

Jenkins, retest this please.

@SparkQA
Copy link

SparkQA commented Apr 17, 2015

Test build #30481 has finished for PR 5551 at commit 4335da1.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.
  • This patch does not change any dependencies.

@vanzin
Copy link
Contributor

vanzin commented Apr 17, 2015

Looks fine to me. Seems like it might be useful (and easy) to have different messages for each case, but not a big deal.

@andrewor14
Copy link
Contributor

This looks fine. The previous setting was very confusing to the user because they would have run into this by following the docs. Thanks for fixing this I'm merging this into master and 1.3

asfgit pushed a commit that referenced this pull request Apr 18, 2015
@jerryshao
[SPARK-6975][Yarn] Fix argument validation error
7d0ab55 
`numExecutors` checking is failed when dynamic allocation is enabled with default configuration. Details can be seen is [SPARK-6975](https://issues.apache.org/jira/browse/SPARK-6975). sryza, please help me to review this, not sure is this the correct way, I think previous you change this part :)

Author: jerryshao <saisai.shao@intel.com>

Closes #5551 from jerryshao/SPARK-6975 and squashes the following commits:

4335da1 [jerryshao] Change according to the comments
77bdcbd [jerryshao] Fix argument validation error

(cherry picked from commit d850b4b)
Signed-off-by: Andrew Or <andrew@databricks.com>
@asfgit asfgit closed this in d850b4b Apr 18, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
5 participants
@jerryshao @SparkQA @srowen @vanzin @andrewor14