Skip to content

[SPARK-7031][ThriftServer]let thrift server take SPARK_DAEMON_MEMORY and SPARK_DAEMON_JAVA_OPTS #5609

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

Closed
WangTaoTheTonic wants to merge 6 commits into from
Closed

[SPARK-7031][ThriftServer]let thrift server take SPARK_DAEMON_MEMORY and SPARK_DAEMON_JAVA_OPTS #5609

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
4fb25ed
let thrift server take SPARK_DAEMON_MEMORY and SPARK_DAEMON_JAVA_OPTS
WangTaoTheTonic Apr 21, 2015
0565831
fix failed tests
WangTaoTheTonic Apr 21, 2015
624e652
fix break tests
WangTaoTheTonic Apr 23, 2015
d3ddfb6
revert the unnecessary changes in suite
WangTaoTheTonic Apr 25, 2015
035069b
better code style
WangTaoTheTonic Apr 30, 2015
8d3fc16
indent
WangTaoTheTonic Apr 30, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 18 additions & 1 deletion launcher/src/main/java/org/apache/spark/launcher/SparkSubmitCommandBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,10 @@ private List<String> buildSparkSubmitCommand(Map<String, String> env) throws IOE
firstNonEmptyValue(SparkLauncher.DRIVER_EXTRA_CLASSPATH, conf, props) : null;

List<String> cmd = buildJavaCommand(extraClassPath);
// Take Thrift Server as daemon
if (isThriftServer(mainClass)) {
Copy link
Member

@srowen srowen Apr 23, 2015

Choose a reason for hiding this comment

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

I'm probably missing basic stuff here but it jumps out at me that you have to special-case thrift server here. Is this really related to spark submit?

Copy link
Contributor Author

@WangTaoTheTonic WangTaoTheTonic Apr 24, 2015

Choose a reason for hiding this comment

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

Yeah. I think so. If we take a look at start-thriftserver.sh then will find in spark-daemon.sh we use spark-submit to submit the org.apache.spark.sql.hive.thriftserver.HiveThriftServer2.

Then it comes into spark-class, and spark-class use org.apache.spark.launcher.Main to resolve args in which it has:

if (className.equals("org.apache.spark.deploy.SparkSubmit")) {
builder = new SparkSubmitCommandBuilder(args);
} else {
builder = new SparkClassCommandBuilder(className, args);
}

So I think we start Thrift Server by spark-submit but at the meantime Thrift Server is sort of daemon process and it should take daemon related options too.

Copy link
Contributor

@vanzin vanzin Apr 25, 2015

Choose a reason for hiding this comment

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

The alternative would be to make this HiveThriftServer2 go through spark-class instead of spark-submit, but I think that would be a bigger change. While I'm not in love of the idea of peppering special cases inside SparkSubmitCommandBuilder, one doesn't sound like a terrible idea. If there are more in the future we can look at a better solution.

Copy link
Contributor Author

@WangTaoTheTonic WangTaoTheTonic Apr 25, 2015

Choose a reason for hiding this comment

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

Total agree with you.

addOptionString(cmd, System.getenv("SPARK_DAEMON_JAVA_OPTS"));
}
addOptionString(cmd, System.getenv("SPARK_SUBMIT_OPTS"));
addOptionString(cmd, System.getenv("SPARK_JAVA_OPTS"));

Expand All @@ -201,7 +205,11 @@ private List<String> buildSparkSubmitCommand(Map<String, String> env) throws IOE
// - SPARK_DRIVER_MEMORY env variable
// - SPARK_MEM env variable
// - default value (512m)
String memory = firstNonEmpty(firstNonEmptyValue(SparkLauncher.DRIVER_MEMORY, conf, props),
// Take Thrift Server as daemon
String tsMemory =
isThriftServer(mainClass) ? System.getenv("SPARK_DAEMON_MEMORY") : null;
String memory = firstNonEmpty(tsMemory,
firstNonEmptyValue(SparkLauncher.DRIVER_MEMORY, conf, props),
System.getenv("SPARK_DRIVER_MEMORY"), System.getenv("SPARK_MEM"), DEFAULT_MEM);
cmd.add("-Xms" + memory);
cmd.add("-Xmx" + memory);
Expand Down Expand Up @@ -292,6 +300,15 @@ private boolean isClientMode(Properties userProps) {
(!userMaster.equals("yarn-cluster") && deployMode == null);
}

/**
* Return whether the given main class represents a thrift server.
*/
private boolean isThriftServer(String mainClass) {
return (mainClass != null &&
mainClass.equals("org.apache.spark.sql.hive.thriftserver.HiveThriftServer2"));
}


private class OptionParser extends SparkSubmitOptionParser {

@Override
Expand Down