Skip to content

[SPARK-20835][Core]It should exit directly when the --total-executor-cores parameter is setted less than 0 when submit a application#18060

Closed
eatoncys wants to merge 7 commits intoapache:masterfrom
eatoncys:totalcores
Closed

[SPARK-20835][Core]It should exit directly when the --total-executor-cores parameter is setted less than 0 when submit a application#18060
eatoncys wants to merge 7 commits intoapache:masterfrom
eatoncys:totalcores

Conversation

@eatoncys
Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

In my test, the submitted app running with out an error when the --total-executor-cores less than 0
and given the warnings:
"2017-05-22 17:19:36,319 WARN org.apache.spark.scheduler.TaskSchedulerImpl: Initial job has not accepted any resources; check your cluster UI to ensure that workers are registered and have sufficient resources";

It should exit directly when the --total-executor-cores parameter is setted less than 0 when submit a application
(Please fill in changes proposed in this fix)

How was this patch tested?

Run the ut tests
(Please explain how this patch was tested. E.g. unit tests, integration tests, manual tests)
(If this patch involves UI changes, please attach a screenshot; otherwise, remove this)

Please review http://spark.apache.org/contributing.html before opening a pull request.

@srowen
Copy link
Copy Markdown
Member

srowen commented May 22, 2017

I agree it should fail faster, but shouldn't many args be validated here? CC @vanzin

@eatoncys
Copy link
Copy Markdown
Contributor Author

@srowen The other parameters are validated at some other places, for example, the --executor-memory parameter is validated at org.apache.spark.memory.UnifiedMemoryManager$.getMaxMemory, the app can exit with error: "java.lang.NumberFormatException" if it is a negative number. But the --total-executor-cores parameter is not validated at anywhere, the app can not exit if it is a negative number.

@srowen
Copy link
Copy Markdown
Member

srowen commented May 23, 2017

Yeah but maybe a good idea to valid all these aren't negative upfront. Fail faster and more consistently.

@eatoncys
Copy link
Copy Markdown
Contributor Author

@srowen Thank you for suggestion. I have validated other numerical parameters here, any other suggestion?

Copy link
Copy Markdown
Member

@srowen srowen left a comment

Choose a reason for hiding this comment

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

Seems reasonable to me. I wonder if @vanzin wants to comment?

@SparkQA
Copy link
Copy Markdown

SparkQA commented May 23, 2017

Test build #3750 has finished for PR 18060 at commit 4c4bfbb.

  • This patch fails Scala style tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@eatoncys
Copy link
Copy Markdown
Contributor Author

Jenkins, retest this please

@eatoncys
Copy link
Copy Markdown
Contributor Author

@SparkQA Retest this please,thanks!

@SparkQA
Copy link
Copy Markdown

SparkQA commented May 25, 2017

Test build #3757 has finished for PR 18060 at commit eae0f3d.

  • This patch fails Scala style tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link
Copy Markdown

SparkQA commented May 26, 2017

Test build #3760 has finished for PR 18060 at commit caacdc0.

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

@srowen
Copy link
Copy Markdown
Member

srowen commented May 26, 2017

Merged to master

@asfgit asfgit closed this in 0fd84b0 May 26, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants