[FLINK-8645][configuration] Split classloader.parent-first-patterns into "base" 
and "append

This closes #5544.


Project: http://git-wip-us.apache.org/repos/asf/flink/repo
Commit: http://git-wip-us.apache.org/repos/asf/flink/commit/50aea889
Tree: http://git-wip-us.apache.org/repos/asf/flink/tree/50aea889
Diff: http://git-wip-us.apache.org/repos/asf/flink/diff/50aea889

Branch: refs/heads/master
Commit: 50aea889bd88d5ce4a1569569176705f9973a08c
Parents: f3bfd22
Author: zentol <[email protected]>
Authored: Wed Feb 21 14:55:22 2018 +0100
Committer: zentol <[email protected]>
Committed: Mon Feb 26 20:42:00 2018 +0100

----------------------------------------------------------------------
 docs/ops/config.md                              | 12 +++--
 .../apache/flink/configuration/CoreOptions.java | 37 ++++++++++++--
 .../flink/configuration/CoreOptionsTest.java    | 54 ++++++++++++++++++++
 .../configuration/ParentFirstPatternsTest.java  |  2 +-
 .../formats/avro/AvroKryoClassloadingTest.java  |  2 +-
 .../jobmaster/JobManagerSharedServices.java     |  4 +-
 .../taskexecutor/TaskManagerConfiguration.java  |  4 +-
 .../flink/runtime/jobmanager/JobManager.scala   |  5 +-
 8 files changed, 101 insertions(+), 19 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/flink/blob/50aea889/docs/ops/config.md
----------------------------------------------------------------------
diff --git a/docs/ops/config.md b/docs/ops/config.md
index efd3ee3..ce5bc9b 100644
--- a/docs/ops/config.md
+++ b/docs/ops/config.md
@@ -75,12 +75,16 @@ without explicit scheme definition, such as 
`/user/USERNAME/in.txt`, is going to
 - `classloader.resolve-order`: Whether Flink should use a child-first 
`ClassLoader` when loading
 user-code classes or a parent-first `ClassLoader`. Can be one of 
`parent-first` or `child-first`. (default: `child-first`)
 
-- `classloader.parent-first-patterns`: A (semicolon-separated) list of 
patterns that specifies which
+- `classloader.parent-first-patterns.default`: 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. By default, 
this is set to
-`java.;org.apache.flink.;javax.annotation;org.slf4j;org.apache.log4j;org.apache.logging.log4j;ch.qos.logback`.
-If you want to change this setting you have to make sure to also include the 
default patterns in
-your list of patterns if you want to keep that default behaviour.
+`"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"`.
+To extend this list beyond the default it is recommended to configure 
`classloader.parent-first-patterns.additional` instead of modifying this 
setting directly.
+
+- `classloader.parent-first-patterns.additional`: 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.
+This list is appended to `classloader.parent-first-patterns.default`.
 
 ## Advanced Options
 

http://git-wip-us.apache.org/repos/asf/flink/blob/50aea889/flink-core/src/main/java/org/apache/flink/configuration/CoreOptions.java
----------------------------------------------------------------------
diff --git 
a/flink-core/src/main/java/org/apache/flink/configuration/CoreOptions.java 
b/flink-core/src/main/java/org/apache/flink/configuration/CoreOptions.java
index 30c0cd6..ccce0ab 100644
--- a/flink-core/src/main/java/org/apache/flink/configuration/CoreOptions.java
+++ b/flink-core/src/main/java/org/apache/flink/configuration/CoreOptions.java
@@ -45,7 +45,7 @@ public class CoreOptions {
         * which means that user code jars can include and load different 
dependencies than
         * Flink uses (transitively).
         *
-        * <p>Exceptions to the rules are defined via {@link 
#ALWAYS_PARENT_FIRST_LOADER}.
+        * <p>Exceptions to the rules are defined via {@link 
#ALWAYS_PARENT_FIRST_LOADER_PATTERNS}.
         */
        public static final ConfigOption<String> CLASSLOADER_RESOLVE_ORDER = 
ConfigOptions
                .key("classloader.resolve-order")
@@ -85,12 +85,41 @@ public class CoreOptions {
         *         log bindings.</li>
         * </ul>
         */
-       public static final ConfigOption<String> ALWAYS_PARENT_FIRST_LOADER = 
ConfigOptions
-               .key("classloader.parent-first-patterns")
+       public static final ConfigOption<String> 
ALWAYS_PARENT_FIRST_LOADER_PATTERNS = ConfigOptions
+               .key("classloader.parent-first-patterns.default")
                
.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.additional\" instead.");
+
+       public static final ConfigOption<String> 
ALWAYS_PARENT_FIRST_LOADER_PATTERNS_ADDITIONAL = ConfigOptions
+               .key("classloader.parent-first-patterns.additional")
+               .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_PATTERNS.key() + "\".");
+
+       public static String[] getParentFirstLoaderPatterns(Configuration 
config) {
+               String base = 
config.getString(ALWAYS_PARENT_FIRST_LOADER_PATTERNS);
+               String append = 
config.getString(ALWAYS_PARENT_FIRST_LOADER_PATTERNS_ADDITIONAL);
+
+               String[] basePatterns = base.isEmpty()
+                       ? new String[0]
+                       : base.split(";");
+
+               if (append.isEmpty()) {
+                       return basePatterns;
+               } else {
+                       String[] appendPatterns = append.split(";");
+
+                       String[] joinedPatterns = new 
String[basePatterns.length + appendPatterns.length];
+                       System.arraycopy(basePatterns, 0, joinedPatterns, 0, 
basePatterns.length);
+                       System.arraycopy(appendPatterns, 0, joinedPatterns, 
basePatterns.length, appendPatterns.length);
+                       return joinedPatterns;
+               }
+       }
 
        // 
------------------------------------------------------------------------
        //  process parameters

http://git-wip-us.apache.org/repos/asf/flink/blob/50aea889/flink-core/src/test/java/org/apache/flink/configuration/CoreOptionsTest.java
----------------------------------------------------------------------
diff --git 
a/flink-core/src/test/java/org/apache/flink/configuration/CoreOptionsTest.java 
b/flink-core/src/test/java/org/apache/flink/configuration/CoreOptionsTest.java
new file mode 100644
index 0000000..0bc5910
--- /dev/null
+++ 
b/flink-core/src/test/java/org/apache/flink/configuration/CoreOptionsTest.java
@@ -0,0 +1,54 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.configuration;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+/**
+ * Tests for {@link CoreOptions}.
+ */
+public class CoreOptionsTest {
+       @Test
+       public void testGetParentFirstLoaderPatterns() {
+               Configuration config = new Configuration();
+
+               Assert.assertArrayEquals(
+                       
CoreOptions.ALWAYS_PARENT_FIRST_LOADER_PATTERNS.defaultValue().split(";"),
+                       CoreOptions.getParentFirstLoaderPatterns(config));
+
+               
config.setString(CoreOptions.ALWAYS_PARENT_FIRST_LOADER_PATTERNS, 
"hello;world");
+
+               Assert.assertArrayEquals(
+                       "hello;world".split(";"),
+                       CoreOptions.getParentFirstLoaderPatterns(config));
+
+               
config.setString(CoreOptions.ALWAYS_PARENT_FIRST_LOADER_PATTERNS_ADDITIONAL, 
"how;are;you");
+
+               Assert.assertArrayEquals(
+                       "hello;world;how;are;you".split(";"),
+                       CoreOptions.getParentFirstLoaderPatterns(config));
+
+               
config.setString(CoreOptions.ALWAYS_PARENT_FIRST_LOADER_PATTERNS, "");
+
+               Assert.assertArrayEquals(
+                       "how;are;you".split(";"),
+                       CoreOptions.getParentFirstLoaderPatterns(config));
+       }
+}

http://git-wip-us.apache.org/repos/asf/flink/blob/50aea889/flink-core/src/test/java/org/apache/flink/configuration/ParentFirstPatternsTest.java
----------------------------------------------------------------------
diff --git 
a/flink-core/src/test/java/org/apache/flink/configuration/ParentFirstPatternsTest.java
 
b/flink-core/src/test/java/org/apache/flink/configuration/ParentFirstPatternsTest.java
index 784d099..ca4b511 100644
--- 
a/flink-core/src/test/java/org/apache/flink/configuration/ParentFirstPatternsTest.java
+++ 
b/flink-core/src/test/java/org/apache/flink/configuration/ParentFirstPatternsTest.java
@@ -34,7 +34,7 @@ import static org.junit.Assert.assertTrue;
 public class ParentFirstPatternsTest extends TestLogger {
 
        private static final HashSet<String> PARENT_FIRST_PACKAGES = new 
HashSet<>(
-                       
Arrays.asList(CoreOptions.ALWAYS_PARENT_FIRST_LOADER.defaultValue().split(";")));
+                       
Arrays.asList(CoreOptions.ALWAYS_PARENT_FIRST_LOADER_PATTERNS.defaultValue().split(";")));
 
        /**
         * All java and Flink classes must be loaded parent first.

http://git-wip-us.apache.org/repos/asf/flink/blob/50aea889/flink-formats/flink-avro/src/test/java/org/apache/flink/formats/avro/AvroKryoClassloadingTest.java
----------------------------------------------------------------------
diff --git 
a/flink-formats/flink-avro/src/test/java/org/apache/flink/formats/avro/AvroKryoClassloadingTest.java
 
b/flink-formats/flink-avro/src/test/java/org/apache/flink/formats/avro/AvroKryoClassloadingTest.java
index 6eaca15..8f0916c 100644
--- 
a/flink-formats/flink-avro/src/test/java/org/apache/flink/formats/avro/AvroKryoClassloadingTest.java
+++ 
b/flink-formats/flink-avro/src/test/java/org/apache/flink/formats/avro/AvroKryoClassloadingTest.java
@@ -73,7 +73,7 @@ public class AvroKryoClassloadingTest {
                final ClassLoader userAppClassLoader = 
FlinkUserCodeClassLoaders.childFirst(
                                new URL[] { avroLocation, kryoLocation },
                                parentClassLoader,
-                               
CoreOptions.ALWAYS_PARENT_FIRST_LOADER.defaultValue().split(";"));
+                               
CoreOptions.ALWAYS_PARENT_FIRST_LOADER_PATTERNS.defaultValue().split(";"));
 
                final Class<?> userLoadedAvroClass = 
Class.forName(avroClass.getName(), false, userAppClassLoader);
                assertNotEquals(avroClass, userLoadedAvroClass);

http://git-wip-us.apache.org/repos/asf/flink/blob/50aea889/flink-runtime/src/main/java/org/apache/flink/runtime/jobmaster/JobManagerSharedServices.java
----------------------------------------------------------------------
diff --git 
a/flink-runtime/src/main/java/org/apache/flink/runtime/jobmaster/JobManagerSharedServices.java
 
b/flink-runtime/src/main/java/org/apache/flink/runtime/jobmaster/JobManagerSharedServices.java
index 34b338e..c1e910c 100644
--- 
a/flink-runtime/src/main/java/org/apache/flink/runtime/jobmaster/JobManagerSharedServices.java
+++ 
b/flink-runtime/src/main/java/org/apache/flink/runtime/jobmaster/JobManagerSharedServices.java
@@ -131,9 +131,7 @@ public class JobManagerSharedServices {
                final String classLoaderResolveOrder =
                        config.getString(CoreOptions.CLASSLOADER_RESOLVE_ORDER);
 
-               final String alwaysParentFirstLoaderString =
-                       
config.getString(CoreOptions.ALWAYS_PARENT_FIRST_LOADER);
-               final String[] alwaysParentFirstLoaderPatterns = 
alwaysParentFirstLoaderString.split(";");
+               final String[] alwaysParentFirstLoaderPatterns = 
CoreOptions.getParentFirstLoaderPatterns(config);
 
                final BlobLibraryCacheManager libraryCacheManager =
                        new BlobLibraryCacheManager(

http://git-wip-us.apache.org/repos/asf/flink/blob/50aea889/flink-runtime/src/main/java/org/apache/flink/runtime/taskexecutor/TaskManagerConfiguration.java
----------------------------------------------------------------------
diff --git 
a/flink-runtime/src/main/java/org/apache/flink/runtime/taskexecutor/TaskManagerConfiguration.java
 
b/flink-runtime/src/main/java/org/apache/flink/runtime/taskexecutor/TaskManagerConfiguration.java
index aebefd6..cb6fe51 100644
--- 
a/flink-runtime/src/main/java/org/apache/flink/runtime/taskexecutor/TaskManagerConfiguration.java
+++ 
b/flink-runtime/src/main/java/org/apache/flink/runtime/taskexecutor/TaskManagerConfiguration.java
@@ -241,9 +241,7 @@ public class TaskManagerConfiguration implements 
TaskManagerRuntimeInfo {
                final String classLoaderResolveOrder =
                        
configuration.getString(CoreOptions.CLASSLOADER_RESOLVE_ORDER);
 
-               final String alwaysParentFirstLoaderString =
-                       
configuration.getString(CoreOptions.ALWAYS_PARENT_FIRST_LOADER);
-               final String[] alwaysParentFirstLoaderPatterns = 
alwaysParentFirstLoaderString.split(";");
+               final String[] alwaysParentFirstLoaderPatterns = 
CoreOptions.getParentFirstLoaderPatterns(configuration);
 
                final String taskManagerLogPath = 
configuration.getString(ConfigConstants.TASK_MANAGER_LOG_PATH_KEY, 
System.getProperty("log.file"));
                final String taskManagerStdoutPath;

http://git-wip-us.apache.org/repos/asf/flink/blob/50aea889/flink-runtime/src/main/scala/org/apache/flink/runtime/jobmanager/JobManager.scala
----------------------------------------------------------------------
diff --git 
a/flink-runtime/src/main/scala/org/apache/flink/runtime/jobmanager/JobManager.scala
 
b/flink-runtime/src/main/scala/org/apache/flink/runtime/jobmanager/JobManager.scala
index 1dfaa5d..b9529de 100644
--- 
a/flink-runtime/src/main/scala/org/apache/flink/runtime/jobmanager/JobManager.scala
+++ 
b/flink-runtime/src/main/scala/org/apache/flink/runtime/jobmanager/JobManager.scala
@@ -2430,9 +2430,8 @@ object JobManager {
     val timeout: FiniteDuration = AkkaUtils.getTimeout(configuration)
 
     val classLoaderResolveOrder = 
configuration.getString(CoreOptions.CLASSLOADER_RESOLVE_ORDER)
-    val alwaysParentFirstLoaderString =
-      configuration.getString(CoreOptions.ALWAYS_PARENT_FIRST_LOADER)
-    val alwaysParentFirstLoaderPatterns = 
alwaysParentFirstLoaderString.split(';')
+
+    val alwaysParentFirstLoaderPatterns = 
CoreOptions.getParentFirstLoaderPatterns(configuration)
 
     val restartStrategy = 
RestartStrategyFactory.createRestartStrategyFactory(configuration)
 

Reply via email to