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

[WangTaoTheTonic]

We should let Thrift Server take these two parameters as it is a daemon. And it is better to read driver-related configs as an app submited by spark-submit.

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

[SparkQA]

Test build #30664 has finished for PR 5609 at commit 4fb25ed.

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

[SparkQA]

Test build #30674 has finished for PR 5609 at commit 0565831.

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

[SparkQA]

Test build #30823 has finished for PR 5609 at commit 624e652.

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

[srowen]

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?

[WangTaoTheTonic]

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.

[vanzin]

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.

[WangTaoTheTonic]

Total agree with you.

[WangTaoTheTonic]

BTW I have tested on my cluster with setting

export SPARK_DAEMON_MEMORY=1111m
export SPARK_DAEMON_JAVA_OPTS=" -Dx=y "

in spark-env.sh.
Before this patch the jinfo shows:

VM Flags:
-Xms512m -Xmx512m -XX:MaxPermSize=128m

After:

VM Flags:
-Dx=y -Xms1111m -Xmx1111m -XX:MaxPermSize=128m

[vanzin]

Are these test changes needed? The tests have been passing without them.

[WangTaoTheTonic]

No they are not necessary and just left while I fixing the broken tests. I will revert these.

[WangTaoTheTonic]

Jenkins, retest this please.

[WangTaoTheTonic]

Jenkins, test this please.

[WangTaoTheTonic]

Jenkins, test this please.

[SparkQA]

Test build #728 has finished for PR 5609 at commit d3ddfb6.

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

[WangTaoTheTonic]

@vanzin So though it is not a best idea but ok to go?

[vanzin]

To avoid the copy & paste, I'd do:

String tsMemory = isThriftServer(mainClass) ? getenv("SPARK_DAEMON_MEMORY") : null;
memory = firstNonEmpty(tsMemory, ...);

[vanzin]

Just a small nit, but otherwise looks good.

[WangTaoTheTonic]

Jenkins, retest this please.

[SparkQA]

Test build #31373 has finished for PR 5609 at commit 8d3fc16.

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

[WangTaoTheTonic]

Jenkins, retest this please.

[SparkQA]

Test build #31378 has finished for PR 5609 at commit 8d3fc16.

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

[WangTaoTheTonic]

Looks like the failed test case is flacky. Jenkins, retest this please.

[SparkQA]

Test build #31384 has finished for PR 5609 at commit 8d3fc16.

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

[WangTaoTheTonic]

Another different case failed. Jenkins, retest this please.

[SparkQA]

Test build #31389 has finished for PR 5609 at commit 8d3fc16.

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

[SparkQA]

Test build #31398 has finished for PR 5609 at commit 8d3fc16.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.
• This patch adds the following new dependencies:
  • jaxb-api-2.2.7.jar
  • jaxb-core-2.2.7.jar
  • jaxb-impl-2.2.7.jar
  • pmml-agent-1.1.15.jar
  • pmml-model-1.1.15.jar
  • pmml-schema-1.1.15.jar
• This patch removes the following dependencies:
  • activation-1.1.jar
  • jaxb-api-2.2.2.jar
  • jaxb-impl-2.2.3-1.jar