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.

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

[SPARK-23830][YARN] added check to ensure main method is found #21168

Closed
wants to merge 4 commits into from
Closed

[SPARK-23830][YARN] added check to ensure main method is found #21168

wants to merge 4 commits into from

Conversation

eric-maynard
Copy link
Contributor

@eric-maynard eric-maynard commented Apr 26, 2018

What changes were proposed in this pull request?

When a user specifies the wrong class -- or, in fact, a class instead of an object -- Spark throws an NPE which is not useful for debugging. This was reported in SPARK-23830. This PR adds a check to ensure the main method was found and logs a useful error in the event that it's null.

How was this patch tested?

  • Unit tests + Manual testing
  • The scope of the changes is very limited

@vanzin
Copy link
Contributor

vanzin commented Apr 26, 2018

The exception happens because mainMethod in the case described in the bug is not a static method (thus the NPE is because of the null instance argument in the call). It's not because the main method does not exist.

Copy link
Contributor

@vanzin vanzin left a comment

Choose a reason for hiding this comment

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

Please fix the title to follow the convention:

[BUG-12345][yarn] Summary.

finish(FinalApplicationStatus.SUCCEEDED, ApplicationMaster.EXIT_SUCCESS)
logDebug("Done running users class")
if(!Modifier.isStatic(mainMethod.getModifiers)) {
logError(s"Could not find main method in object ${args.userClass}")
Copy link
Contributor

Choose a reason for hiding this comment

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

The message could be a little more helpful here.

mainMethod.invoke(null, userArgs.toArray)
finish(FinalApplicationStatus.SUCCEEDED, ApplicationMaster.EXIT_SUCCESS)
logDebug("Done running users class")
if(!Modifier.isStatic(mainMethod.getModifiers)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This won't pass the style checker...

@HyukjinKwon
Copy link
Member

ok to test

@SparkQA
Copy link

SparkQA commented Apr 27, 2018

Test build #89910 has finished for PR 21168 at commit 571ae40.

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

@eric-maynard eric-maynard changed the title added check to ensure main method is found [SPARK-23830] [SPARK-23830][CORE] added check to ensure main method is found Apr 27, 2018
added a space after an `if`
@SparkQA
Copy link

SparkQA commented Apr 27, 2018

Test build #89911 has finished for PR 21168 at commit 3d90a5e.

  • This patch fails to build.
  • This patch merges cleanly.
  • This patch adds no public classes.

@jerryshao
Copy link
Contributor

The change is fail to build, please fix it.

@vanzin
Copy link
Contributor

vanzin commented Apr 27, 2018

Also this is a yarn, not core, change.

@eric-maynard eric-maynard changed the title [SPARK-23830][CORE] added check to ensure main method is found [SPARK-23830][YARN] added check to ensure main method is found Apr 27, 2018
added an import for `java.lang.reflect.Modifier`
@SparkQA
Copy link

SparkQA commented Apr 27, 2018

Test build #89913 has finished for PR 21168 at commit 10a6232.

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

Copy link
Contributor

@jerryshao jerryshao left a comment

Choose a reason for hiding this comment

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

LGTM. Merging to master branch.

@asfgit asfgit closed this in 109935f Apr 27, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants