[GitHub] flink pull request #5544: [FLINK-8645][configuration] Split classloader.pare...
Github user asfgit closed the pull request at: https://github.com/apache/flink/pull/5544 ---
[GitHub] flink pull request #5544: [FLINK-8645][configuration] Split classloader.pare...
Github user zentol commented on a diff in the pull request:
https://github.com/apache/flink/pull/5544#discussion_r169699380
--- Diff:
flink-core/src/main/java/org/apache/flink/configuration/CoreOptions.java ---
@@ -86,11 +86,42 @@
*
*/
public static final ConfigOption ALWAYS_PARENT_FIRST_LOADER =
ConfigOptions
- .key("classloader.parent-first-patterns")
+ .key("classloader.parent-first-patterns.base")
.defaultValue("java.;scala.;org.apache.flink.;com.esotericsoftware.kryo;org.apache.hadoop.;javax.annotation.;org.slf4j;org.apache.log4j;org.apache.logging.log4j;ch.qos.logback")
+ .withDeprecatedKeys("classloader.parent-first-patterns")
.withDescription("A (semicolon-separated) list of patterns that
specifies which classes should always be" +
" resolved through the parent ClassLoader first. A
pattern is a simple prefix that is checked against" +
- " the fully qualified class name.");
+ " the fully qualified class name. This setting should
generally not be modified. To add another pattern we" +
+ " recommend to use
\"classloader.parent-first-patterns.append\" instead.");
+
+ public static final ConfigOption
ALWAYS_PARENT_FIRST_LOADER_APPEND = ConfigOptions
+ .key("classloader.parent-first-patterns.append")
+ .defaultValue("")
+ .withDescription("A (semicolon-separated) list of patterns that
specifies which classes should always be" +
+ " resolved through the parent ClassLoader first. A
pattern is a simple prefix that is checked against" +
+ " the fully qualified class name. These patterns are
appended to \"" + ALWAYS_PARENT_FIRST_LOADER.key() + "\".");
+
+ private static final String[] EMPTY_STRING_ARRAY = new String[0];
--- End diff --
I've removed the static field.
---
[GitHub] flink pull request #5544: [FLINK-8645][configuration] Split classloader.pare...
Github user StephanEwen commented on a diff in the pull request:
https://github.com/apache/flink/pull/5544#discussion_r169660742
--- Diff:
flink-core/src/main/java/org/apache/flink/configuration/CoreOptions.java ---
@@ -86,11 +86,42 @@
*
*/
public static final ConfigOption ALWAYS_PARENT_FIRST_LOADER =
ConfigOptions
- .key("classloader.parent-first-patterns")
+ .key("classloader.parent-first-patterns.base")
.defaultValue("java.;scala.;org.apache.flink.;com.esotericsoftware.kryo;org.apache.hadoop.;javax.annotation.;org.slf4j;org.apache.log4j;org.apache.logging.log4j;ch.qos.logback")
+ .withDeprecatedKeys("classloader.parent-first-patterns")
.withDescription("A (semicolon-separated) list of patterns that
specifies which classes should always be" +
" resolved through the parent ClassLoader first. A
pattern is a simple prefix that is checked against" +
- " the fully qualified class name.");
+ " the fully qualified class name. This setting should
generally not be modified. To add another pattern we" +
+ " recommend to use
\"classloader.parent-first-patterns.append\" instead.");
+
+ public static final ConfigOption
ALWAYS_PARENT_FIRST_LOADER_APPEND = ConfigOptions
+ .key("classloader.parent-first-patterns.append")
+ .defaultValue("")
+ .withDescription("A (semicolon-separated) list of patterns that
specifies which classes should always be" +
+ " resolved through the parent ClassLoader first. A
pattern is a simple prefix that is checked against" +
+ " the fully qualified class name. These patterns are
appended to \"" + ALWAYS_PARENT_FIRST_LOADER.key() + "\".");
+
+ private static final String[] EMPTY_STRING_ARRAY = new String[0];
--- End diff --
This is meant well (as in fewer objects), but given that this is used only
once (or few times), the JVM has more overhead through this additional static
field (managing class and reflection metadata) than if the method
`getParentFirstLoaderPatterns()` just does a `new String[0]`, especially
because it will be a very short lived object.
---
[GitHub] flink pull request #5544: [FLINK-8645][configuration] Split classloader.pare...
GitHub user zentol opened a pull request: https://github.com/apache/flink/pull/5544 [FLINK-8645][configuration] Split classloader.parent-first-patterns into "base" and "append ## What is the purpose of the change This PR splits the `classloader.parent-first-patterns` option into a `base` and `append` to allow users to specify additional patterns without having to include the default as well. ## Brief change log * change key of `ALWAYS_PARENT_FIRST_LOADER` option by appending `.base` * add deprecated key `classloader.parent-first-patterns` to not break existing configs * add `ALWAYS_PARENT_FIRST_LOADER_APPEND` option * add utility method `getParentFirstLoaderPatterns` to `CoreOptions` for parsing both the base and append at once from a given configuration * migrate all production accesses of `ALWAYS_PARENT_FIRST_LOADER` to the utility method * add a test for `getParentFirstLoaderPatterns` * update documentation * update default pattern * explicitly recommend the append option for extending the pattern list ## Verifying this change This change added tests and can be verified as follows: * run `CoreOptionsTest` ## Does this pull request potentially affect one of the following parts: - Dependencies (does it add or upgrade a dependency): (no) - The public API, i.e., is any changed class annotated with `@Public(Evolving)`: (yes) - The serializers: (ni) - The runtime per-record code paths (performance sensitive): (ni) - Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Yarn/Mesos, ZooKeeper: (no) - The S3 file system connector: (no) ## Documentation - Does this pull request introduce a new feature? (yes) - If yes, how is the feature documented? (docs) You can merge this pull request into a Git repository by running: $ git pull https://github.com/zentol/flink 8645 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/flink/pull/5544.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 #5544 commit 6bf6c6478f7cfbf392eb2cc965651936af2fc606 Author: zentol Date: 2018-02-21T13:55:22Z [FLINK-8645][configuration] Split classloader.parent-first-patterns into "base" and "append ---
