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-3658][SQL]Take thrift server as a daemon #2509

Closed
wants to merge 3 commits into from

Conversation

WangTaoTheTonic
Copy link
Contributor

https://issues.apache.org/jira/browse/SPARK-3658

And keep the CLASS_NOT_FOUND_EXIT_STATUS and exit message in SparkSubmit.scala.

@SparkQA
Copy link

SparkQA commented Sep 23, 2014

QA tests have started for PR 2509 at commit 598e21e.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Sep 23, 2014

QA tests have finished for PR 2509 at commit 598e21e.

  • This patch passes unit tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • println(s"Failed to load main class $childMainClass.")
    • logInfo("Interrupting user class to stop.")

@SparkQA
Copy link

SparkQA commented Sep 23, 2014

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/20710/

@WangTaoTheTonic WangTaoTheTonic changed the title [SPARK-3658]Take thrift server as a daemon [SPARK-3658][SQL]Take thrift server as a daemon Sep 24, 2014
@pwendell
Copy link
Contributor

@liancheng - can you take a look at this one?

if (childMainClass.contains("thriftserver")) {
println(s"Failed to load main class $childMainClass.")
println("You need to build Spark with -Phive.")
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I feel complicated to add this logic here... SparkSubmit should not be coupled with Thrift server, but it seems that this is the only legitimate place.

Copy link
Contributor

Choose a reason for hiding this comment

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

After some thoughts, I think it's OK to add this checking here. Mainly because it avoids using the exit code checking trick, which may cause potential exit code conflict.

@liancheng
Copy link
Contributor

Generally this is a good idea. But it would be better to make spark-daemon.sh more general, rather than making HiveThriftServer2 a special case.

@SparkQA
Copy link

SparkQA commented Sep 25, 2014

QA tests have started for PR 2509 at commit 8ad9f95.

  • This patch merges cleanly.

@WangTaoTheTonic
Copy link
Contributor Author

@liancheng As you said, I put "spark-submit" as an option to achieve generalization and use source(dot) instead of exec to make SUBMISSION_OPTS and APPLICATION_OPTS still work in spark-daemon.sh.

As for the ClassNotFound error message, we could either do like this or just delete it?

please check.thanks

@SparkQA
Copy link

SparkQA commented Sep 25, 2014

QA tests have finished for PR 2509 at commit 8ad9f95.

  • This patch passes unit tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • println(s"Failed to load main class $childMainClass.")

@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/20807/

@@ -0,0 +1,25 @@
#!/usr/bin/env bash
Copy link
Contributor

Choose a reason for hiding this comment

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

This file should be executable, please chmod +x.

@liancheng
Copy link
Contributor

Thanks for working on this! I tested this PR locally and it works fine. But there are still some minor issues pending to be resolved, please refer to the comments for details.

@WangTaoTheTonic
Copy link
Contributor Author

@liancheng I tried export and it worked. Thanks for the suggestion.
Also modified permission of stop-thriftserver.sh.

@AmplabJenkins
Copy link

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/20937/

@SparkQA
Copy link

SparkQA commented Sep 28, 2014

QA tests have started for PR 2509 at commit 5dcaab2.

  • This patch merges cleanly.

@liancheng
Copy link
Contributor

After updating my local repo, I found that stop-thriftserver.sh is still not executable. Make sure to git add this file after chmod +x. This is the only pending issue from my perspective.

@SparkQA
Copy link

SparkQA commented Sep 28, 2014

Tests timed out after a configured wait of 120m.

@WangTaoTheTonic
Copy link
Contributor Author

eh...I cloned the repository on another laptop and found it's executable, as shown in top-left corner of https://github.com/WangTaoTheTonic/spark/blob/thriftserver/sbin/stop-thriftserver.sh.

Could you verify this again?

@liancheng
Copy link
Contributor

Ah, sorry, my fault. Then this LGTM, thanks!

@pwendell
Copy link
Contributor

Jenkins, test this please.

@SparkQA
Copy link

SparkQA commented Sep 29, 2014

QA tests have started for PR 2509 at commit 5dcaab2.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Sep 29, 2014

QA tests have finished for PR 2509 at commit 5dcaab2.

  • This patch passes unit tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • println(s"Failed to load main class $childMainClass.")

@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/20958/

@marmbrus
Copy link
Contributor

marmbrus commented Oct 1, 2014

Thanks! I've merged this to master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants