You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by cl4es <gi...@git.apache.org> on 2016/02/10 23:45:55 UTC
[GitHub] spark pull request: [SPARK-13278][CORE] Launcher fails to start wi...
GitHub user cl4es opened a pull request:
https://github.com/apache/spark/pull/11160
[SPARK-13278][CORE] Launcher fails to start with JDK 9 EA
See http://openjdk.java.net/jeps/223 for more information about the JDK 9 version string scheme.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/cl4es/spark master
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/11160.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #11160
----
commit 0da18a66a1278cf8cceefaa53f0cd97a7c75899d
Author: Claes Redestad <cl...@gmail.com>
Date: 2016-02-10T22:38:46Z
[SPARK-13278][CORE] Launcher fails to start with JDK 9 EA
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13278][CORE] Launcher fails to start wi...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/11160#discussion_r52825264
--- Diff: project/SparkBuild.scala ---
@@ -167,8 +167,8 @@ object SparkBuild extends PomBuild {
publishLocalBoth <<= Seq(publishLocal in MavenCompile, publishLocal).dependOn,
javacOptions in (Compile, doc) ++= {
- val Array(major, minor, _) = System.getProperty("java.version").split("\\.", 3)
- if (major.toInt >= 1 && minor.toInt >= 8) Seq("-Xdoclint:all", "-Xdoclint:-missing") else Seq.empty
+ val version = System.getProperty("java.version")
+ if (version >= "1.8.0") Seq("-Xdoclint:all", "-Xdoclint:-missing") else Seq.empty
--- End diff --
Hm, this only happens to work for 1.8 vs 9. This seems hacky compared to just emulating the simple logic you wrote above -- why not that?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13278][CORE] Launcher fails to start wi...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/11160#issuecomment-182617883
Can one of the admins verify this patch?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13278][CORE] Launcher fails to start wi...
Posted by zsxwing <gi...@git.apache.org>.
Github user zsxwing commented on the pull request:
https://github.com/apache/spark/pull/11160#issuecomment-182638883
ok to test
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13278][CORE] Launcher fails to start wi...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/11160
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13278][CORE] Launcher fails to start wi...
Posted by zsxwing <gi...@git.apache.org>.
Github user zsxwing commented on the pull request:
https://github.com/apache/spark/pull/11160#issuecomment-182639087
LGTM pending tests.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13278][CORE] Launcher fails to start wi...
Posted by cl4es <gi...@git.apache.org>.
Github user cl4es commented on a diff in the pull request:
https://github.com/apache/spark/pull/11160#discussion_r52550759
--- Diff: launcher/src/main/java/org/apache/spark/launcher/CommandBuilderUtils.java ---
@@ -336,4 +334,18 @@ static void addPermGenSizeOpt(List<String> cmd) {
cmd.add("-XX:MaxPermSize=256m");
}
+ /**
+ * Get the major version of the java.version string supplied.
+ */
+ static int javaMajorVersion(String javaVersion) {
+ String[] version = javaVersion.split("[+.\\-]+");
--- End diff --
Ah, yes, you're right that handling + is probably superfluous for java.version. I can prune that from the commit if you want, but doesn't seem to do any harm to leave it in for completeness.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13278][CORE] Launcher fails to start wi...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/11160#issuecomment-183581495
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/51221/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13278][CORE] Launcher fails to start wi...
Posted by zsxwing <gi...@git.apache.org>.
Github user zsxwing commented on a diff in the pull request:
https://github.com/apache/spark/pull/11160#discussion_r52547538
--- Diff: launcher/src/main/java/org/apache/spark/launcher/CommandBuilderUtils.java ---
@@ -336,4 +334,18 @@ static void addPermGenSizeOpt(List<String> cmd) {
cmd.add("-XX:MaxPermSize=256m");
}
+ /**
+ * Get the major version of the java.version string supplied.
+ */
+ static int javaMajorVersion(String javaVersion) {
+ String[] version = javaVersion.split("[+.\\-]+");
--- End diff --
But for early access version, e.g., `9-ea`, the previous codes will throw an exception.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13278][CORE] Launcher fails to start wi...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/11160#issuecomment-182675734
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/51068/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13278][CORE] Launcher fails to start wi...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/11160#issuecomment-183581421
**[Test build #51221 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/51221/consoleFull)** for PR 11160 at commit [`68159d8`](https://github.com/apache/spark/commit/68159d88de7a66592b4a0e4384d8cb2a29e298f8).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13278][CORE] Launcher fails to start wi...
Posted by holdenk <gi...@git.apache.org>.
Github user holdenk commented on a diff in the pull request:
https://github.com/apache/spark/pull/11160#discussion_r52546778
--- Diff: launcher/src/main/java/org/apache/spark/launcher/CommandBuilderUtils.java ---
@@ -336,4 +334,18 @@ static void addPermGenSizeOpt(List<String> cmd) {
cmd.add("-XX:MaxPermSize=256m");
}
+ /**
+ * Get the major version of the java.version string supplied.
+ */
+ static int javaMajorVersion(String javaVersion) {
+ String[] version = javaVersion.split("[+.\\-]+");
--- End diff --
this is a super minor point, but I don't think we need to split on anything other than "." even with non-JEP232 strings since we only want to extract the major version.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13278][CORE] Launcher fails to start wi...
Posted by zsxwing <gi...@git.apache.org>.
Github user zsxwing commented on a diff in the pull request:
https://github.com/apache/spark/pull/11160#discussion_r52549918
--- Diff: launcher/src/main/java/org/apache/spark/launcher/CommandBuilderUtils.java ---
@@ -336,4 +334,18 @@ static void addPermGenSizeOpt(List<String> cmd) {
cmd.add("-XX:MaxPermSize=256m");
}
+ /**
+ * Get the major version of the java.version string supplied.
+ */
+ static int javaMajorVersion(String javaVersion) {
+ String[] version = javaVersion.split("[+.\\-]+");
--- End diff --
I meant `Integer.parseInt("9-ea")` will throw `NumberFormatException`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13278][CORE] Launcher fails to start wi...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/11160#issuecomment-182675732
Merged build finished. Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13278][CORE] Launcher fails to start wi...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/11160#issuecomment-183663172
**[Test build #51241 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/51241/consoleFull)** for PR 11160 at commit [`8220484`](https://github.com/apache/spark/commit/82204845a79874abbb67f63d56366c31a7ed2be4).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13278][CORE] Launcher fails to start wi...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the pull request:
https://github.com/apache/spark/pull/11160#issuecomment-183877799
LGTM
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13278][CORE] Launcher fails to start wi...
Posted by holdenk <gi...@git.apache.org>.
Github user holdenk commented on a diff in the pull request:
https://github.com/apache/spark/pull/11160#discussion_r52550348
--- Diff: launcher/src/main/java/org/apache/spark/launcher/CommandBuilderUtils.java ---
@@ -336,4 +334,18 @@ static void addPermGenSizeOpt(List<String> cmd) {
cmd.add("-XX:MaxPermSize=256m");
}
+ /**
+ * Get the major version of the java.version string supplied.
+ */
+ static int javaMajorVersion(String javaVersion) {
+ String[] version = javaVersion.split("[+.\\-]+");
--- End diff --
oh yah sorry not enough coffee for me today, I guess we need .- but not + (although if we did use java.runtime.version we would need the +) and one of the samples in the test "9+100" shouldn't show up in java.version but should in java.runtime.version
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13278][CORE] Launcher fails to start wi...
Posted by cl4es <gi...@git.apache.org>.
Github user cl4es commented on the pull request:
https://github.com/apache/spark/pull/11160#issuecomment-183558848
While not a general rule, testing lexically that `version >= "1.8.0"` like in `UtilsSuite` is actually OK for all current and all possible JEP-223 versions. I fixed `SparkBuild.scala` to use this trick, and gave the comment to `javaMajorVersion` an overhaul.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13278][CORE] Launcher fails to start wi...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the pull request:
https://github.com/apache/spark/pull/11160#issuecomment-183625244
I think `UtilsSuite` still needs a treatment.
I think the code and scripts attempt to not set MaxPermSize for Java 8 already. If it doesn't somewhere, that also needs to be patched up. But my impression is this should already be the case
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13278][CORE] Launcher fails to start wi...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the pull request:
https://github.com/apache/spark/pull/11160#issuecomment-183877997
Merged to master
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13278][CORE] Launcher fails to start wi...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/11160#issuecomment-183581494
Merged build finished. Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13278][CORE] Launcher fails to start wi...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/11160#issuecomment-183685816
**[Test build #51241 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/51241/consoleFull)** for PR 11160 at commit [`8220484`](https://github.com/apache/spark/commit/82204845a79874abbb67f63d56366c31a7ed2be4).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13278][CORE] Launcher fails to start wi...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the pull request:
https://github.com/apache/spark/pull/11160#issuecomment-182779157
I think `SparkBuild.scala` has a similar computation that needs a similar treatment. Also `test("Kill process")` in `UtilsSuite.scala`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13278][CORE] Launcher fails to start wi...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/11160#issuecomment-183685861
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/51241/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13278][CORE] Launcher fails to start wi...
Posted by holdenk <gi...@git.apache.org>.
Github user holdenk commented on a diff in the pull request:
https://github.com/apache/spark/pull/11160#discussion_r52551738
--- Diff: launcher/src/main/java/org/apache/spark/launcher/CommandBuilderUtils.java ---
@@ -336,4 +334,18 @@ static void addPermGenSizeOpt(List<String> cmd) {
cmd.add("-XX:MaxPermSize=256m");
}
+ /**
+ * Get the major version of the java.version string supplied.
+ */
+ static int javaMajorVersion(String javaVersion) {
+ String[] version = javaVersion.split("[+.\\-]+");
--- End diff --
It probably isn't much harm; the only concern would be that it split string, the test and the comment together imply a different input format than the one specified. It might be slightly nicer to have it match the spec since its just a few characters difference. (but a super minor point)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13278][CORE] Launcher fails to start wi...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/11160#issuecomment-183559586
**[Test build #51221 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/51221/consoleFull)** for PR 11160 at commit [`68159d8`](https://github.com/apache/spark/commit/68159d88de7a66592b4a0e4384d8cb2a29e298f8).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13278][CORE] Launcher fails to start wi...
Posted by holdenk <gi...@git.apache.org>.
Github user holdenk commented on a diff in the pull request:
https://github.com/apache/spark/pull/11160#discussion_r52549324
--- Diff: launcher/src/main/java/org/apache/spark/launcher/CommandBuilderUtils.java ---
@@ -336,4 +334,18 @@ static void addPermGenSizeOpt(List<String> cmd) {
cmd.add("-XX:MaxPermSize=256m");
}
+ /**
+ * Get the major version of the java.version string supplied.
+ */
+ static int javaMajorVersion(String javaVersion) {
+ String[] version = javaVersion.split("[+.\\-]+");
--- End diff --
But they won't since we only try and access the second element of the array if the first element is less than or equal to 1.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13278][CORE] Launcher fails to start wi...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/11160#issuecomment-183685860
Merged build finished. Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13278][CORE] Launcher fails to start wi...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/11160#issuecomment-182675372
**[Test build #51068 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/51068/consoleFull)** for PR 11160 at commit [`0da18a6`](https://github.com/apache/spark/commit/0da18a66a1278cf8cceefaa53f0cd97a7c75899d).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13278][CORE] Launcher fails to start wi...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/11160#issuecomment-182642640
**[Test build #51068 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/51068/consoleFull)** for PR 11160 at commit [`0da18a6`](https://github.com/apache/spark/commit/0da18a66a1278cf8cceefaa53f0cd97a7c75899d).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13278][CORE] Launcher fails to start wi...
Posted by cl4es <gi...@git.apache.org>.
Github user cl4es commented on a diff in the pull request:
https://github.com/apache/spark/pull/11160#discussion_r52549879
--- Diff: launcher/src/main/java/org/apache/spark/launcher/CommandBuilderUtils.java ---
@@ -336,4 +334,18 @@ static void addPermGenSizeOpt(List<String> cmd) {
cmd.add("-XX:MaxPermSize=256m");
}
+ /**
+ * Get the major version of the java.version string supplied.
+ */
+ static int javaMajorVersion(String javaVersion) {
+ String[] version = javaVersion.split("[+.\\-]+");
--- End diff --
For `9-ea` the original split would give you an array whose first element is "9-ea", Integer.parseInt("9-ea") throws NumberFormatException. Feel free to try revert to the old regex and run the test cases I added to verify this.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13278][CORE] Launcher fails to start wi...
Posted by zsxwing <gi...@git.apache.org>.
Github user zsxwing commented on the pull request:
https://github.com/apache/spark/pull/11160#issuecomment-183513433
@cl4es could you also fix the places mentioned by @srowen ?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13278][CORE] Launcher fails to start wi...
Posted by cl4es <gi...@git.apache.org>.
Github user cl4es commented on the pull request:
https://github.com/apache/spark/pull/11160#issuecomment-183523227
I tried building with JDK 9 EA, and there are more immediate issues such as -XX:MaxPermSize (which was deprecated/removed in 8 and fails to start in 9) being used in various places. The quick fix would be to add `-XX:+IgnoreUnrecognizedVMOptions`, but that might be unsanitary. I can give fixing the version string logic a shot, though.
Runtime support is of a more immediate concern to us, though, as it allows the OpenJDK project to use Spark itself as a functional test/benchmark. Sadly JEP-223 has stopped that effort for now.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13278][CORE] Launcher fails to start wi...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/11160#discussion_r52577418
--- Diff: launcher/src/main/java/org/apache/spark/launcher/CommandBuilderUtils.java ---
@@ -336,4 +334,18 @@ static void addPermGenSizeOpt(List<String> cmd) {
cmd.add("-XX:MaxPermSize=256m");
}
+ /**
+ * Get the major version of the java.version string supplied.
+ */
+ static int javaMajorVersion(String javaVersion) {
+ String[] version = javaVersion.split("[+.\\-]+");
--- End diff --
How about just splitting on non-numbers? It's all kind of a theoretical difference though. This looks OK.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-13278][CORE] Launcher fails to start wi...
Posted by cl4es <gi...@git.apache.org>.
Github user cl4es commented on the pull request:
https://github.com/apache/spark/pull/11160#issuecomment-183662509
This should likely be refactored to a place where both sbt and test code can access it, but I hope this is good enough.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org