-
Notifications
You must be signed in to change notification settings - Fork 8
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
ISSUE-598: Fix SR to run on Java 11 #599
Conversation
kamalcph
commented
Oct 1, 2019
- Used G1GC instead of parallel GC.
- Included JAXB-API and it's runtime jars to run the JDK8 compiled build on JRE11.
This PR is built on top of #597 |
@kamalcph Can you rebase this PR with latest MASTER ? |
* Used G1GC instead of parallel GC. * Included JAXB-API and it's runtime jars to run the JDK8 compiled build on JRE11.
@raju-saravanan |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall LGTM, left few minor comments.
# GC options | ||
GC_LOG_FILE_NAME='registry-gc.log' | ||
if [ -z "$REGISTRY_GC_LOG_OPTS" ]; then | ||
JAVA_MAJOR_VERSION=$($JAVA -version 2>&1 | sed -E -n 's/.* version "([0-9]*).*$/\1/p') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit : Shouldn't it be "([0-9]+) instead of "([0-9]*) ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Both of them works!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kamalcph It's a nit :)
System.out.println("Failed to load " + jarFile + " into classpath."); | ||
throw new Exception(e); | ||
} | ||
} | ||
|
||
static class MyURLClassLoader extends URLClassLoader { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
May be we can call this JarClassLoader or some other better name ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Renamed to JarClassLoader.
String bootstrapDirPath, | ||
String mysqlJarUrl, | ||
Proxy proxy) throws Exception { | ||
public static ClassLoader downloadMySQLJarIfNeeded(StorageProviderConfiguration storageProperties, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
May be we can call this loadMySQLJarIfNeeded() ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Renamed to maybeLoadMySQLJar
.
* Addressed the review comments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 LGTM