[SPARK-3425] do not set MaxPermSize for OpenJDK 1.8 #2301
Conversation
|
QA tests have started for PR 2301 at commit
|
|
QA tests have finished for PR 2301 at commit
|
| @@ -105,7 +105,7 @@ else | |||
| exit 1 | |||
| fi | |||
| fi | |||
| JAVA_VERSION=$($RUNNER -version 2>&1 | sed 's/java version "\(.*\)\.\(.*\)\..*"/\1\2/; 1q') | |||
| JAVA_VERSION=$($RUNNER -version 2>&1 | sed 's/java\|openjdk version "\(.*\)\.\(.*\)\..*"/\1\2/; 1q') | |||
There was a problem hiding this comment.
This regex seems wrong.. it now breaks the case when you print "java version X". You need parens around the (java\|openjdk). Also, are you sure that OpenJDK 1.8 prints this string instead of "java version X"? I tried 1.6 and it still prints "java".
There was a problem hiding this comment.
nice catch. i had tried parens, but they don't work. it'll have to be a .*
$ java -version
openjdk version "1.8.0_11"
OpenJDK Runtime Environment (build 1.8.0_11-b12)
OpenJDK 64-Bit Server VM (build 25.11-b02, mixed mode)
$ echo 'openjdk version "1.8.0_11"' | sed 's/java\|openjdk version "\(.*\)\.\(.*\)\..*"/\1\2/; 1q'
18
$ echo 'java version "1.8.0_11"' | sed 's/java\|openjdk version "\(.*\)\.\(.*\)\..*"/\1\2/; 1q'
version "1.8.0_11"
$ echo 'java version "1.8.0_11"' | sed 's/(java\|openjdk) version "\(.*\)\.\(.*\)\..*"/\1\2/; 1q'
java version "1.8.0_11"
$ echo 'openjdk version "1.8.0_11"' | sed 's/(java\|openjdk) version "\(.*\)\.\(.*\)\..*"/\1\2/; 1q'
openjdk version "1.8.0_11"
$ echo 'java version "1.8.0_11"' | sed 's/.* version "\(.*\)\.\(.*\)\..*"/\1\2/; 1q'
18
$ echo 'openjdk version "1.8.0_11"' | sed 's/.* version "\(.*\)\.\(.*\)\..*"/\1\2/; 1q'
18
There was a problem hiding this comment.
It would work if you add backslahes before the open and close parentheses and increment the final numbers to \2 and \3.
|
QA tests have started for PR 2301 at commit
|
|
QA tests have finished for PR 2301 at commit
|
failures appear to be from mima and hive, unrelated to this patch |
|
Jenkins, retest this please. |
|
QA tests have started for PR 2301 at commit
|
|
rebased on new master jenkins, retest this please |
|
QA tests have finished for PR 2301 at commit
|
rebased already, please try again |
|
LGTM (@mattf I was asked to check it out.) I only tested this patch with Java 1.7.0_60. |
|
Jenkins, test this please. LGTM pending tests. |
|
Note: need to add "Closes #2387" to the description. |
|
QA tests have started for PR 2301 at commit
|
|
QA tests have finished for PR 2301 at commit
|
Closes #2387