You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@flink.apache.org by se...@apache.org on 2018/01/19 12:41:11 UTC

[1/3] flink git commit: [FLINK-8455] [core] Make 'org.apache.hadoop.' a 'parent-first' classloading pattern.

Repository: flink
Updated Branches:
  refs/heads/master 14840809b -> d57215497


[FLINK-8455] [core] Make 'org.apache.hadoop.' a 'parent-first' classloading pattern.

This change avoid duplication of Hadoop classes between the Flink runtime and the user code.
Hadoop (and transitively its dependencies) should be part of the application class loader.
The user code classloader is allowed to duplicate transitive dependencies, but not Hadoop's
classes directly.

This also adds tests to validate parent-first classloading patterns.


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

Branch: refs/heads/master
Commit: d57215497f36f87939c1cdf3b090d00d8e59d90b
Parents: f9aff59
Author: Stephan Ewen <se...@apache.org>
Authored: Thu Jan 18 17:57:10 2018 +0100
Committer: Stephan Ewen <se...@apache.org>
Committed: Fri Jan 19 13:40:00 2018 +0100

----------------------------------------------------------------------
 .../apache/flink/configuration/CoreOptions.java |  2 +-
 .../configuration/ParentFirstPatternsTest.java  | 78 ++++++++++++++++++++
 2 files changed, 79 insertions(+), 1 deletion(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/flink/blob/d5721549/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 1182ed5..cd93b99 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
@@ -79,7 +79,7 @@ public class CoreOptions {
 	 */
 	public static final ConfigOption<String> ALWAYS_PARENT_FIRST_LOADER = ConfigOptions
 		.key("classloader.parent-first-patterns")
-		.defaultValue("java.;scala.;org.apache.flink.;javax.annotation;org.slf4j;org.apache.log4j;org.apache.logging.log4j;ch.qos.logback");
+		.defaultValue("java.;scala.;org.apache.flink.;org.apache.hadoop.;javax.annotation.;org.slf4j;org.apache.log4j;org.apache.logging.log4j;ch.qos.logback");
 
 	// ------------------------------------------------------------------------
 	//  process parameters

http://git-wip-us.apache.org/repos/asf/flink/blob/d5721549/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
new file mode 100644
index 0000000..784d099
--- /dev/null
+++ b/flink-core/src/test/java/org/apache/flink/configuration/ParentFirstPatternsTest.java
@@ -0,0 +1,78 @@
+/*
+ * 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.apache.flink.util.TestLogger;
+
+import org.junit.Test;
+
+import java.util.Arrays;
+import java.util.HashSet;
+
+import static org.junit.Assert.assertTrue;
+
+/**
+ * Test that checks that all packages that need to be loaded 'parent-first' are also
+ * in the parent-first patterns.
+ */
+public class ParentFirstPatternsTest extends TestLogger {
+
+	private static final HashSet<String> PARENT_FIRST_PACKAGES = new HashSet<>(
+			Arrays.asList(CoreOptions.ALWAYS_PARENT_FIRST_LOADER.defaultValue().split(";")));
+
+	/**
+	 * All java and Flink classes must be loaded parent first.
+	 */
+	@Test
+	public void testAllCorePatterns() {
+		assertTrue(PARENT_FIRST_PACKAGES.contains("java."));
+		assertTrue(PARENT_FIRST_PACKAGES.contains("org.apache.flink."));
+		assertTrue(PARENT_FIRST_PACKAGES.contains("javax.annotation."));
+	}
+
+	/**
+	 * To avoid multiple binding problems and warnings for logger frameworks, we load them
+	 * parent-first.
+	 */
+	@Test
+	public void testLoggersParentFirst() {
+		assertTrue(PARENT_FIRST_PACKAGES.contains("org.slf4j"));
+		assertTrue(PARENT_FIRST_PACKAGES.contains("org.apache.log4j"));
+		assertTrue(PARENT_FIRST_PACKAGES.contains("org.apache.logging.log4j"));
+		assertTrue(PARENT_FIRST_PACKAGES.contains("ch.qos.logback"));
+	}
+
+	/**
+	 * As long as Scala is not a pure user library, but is also used in the Flink runtime, we need
+	 * to load all Scala classes parent-first.
+	 */
+	@Test
+	public void testScalaParentFirst() {
+		assertTrue(PARENT_FIRST_PACKAGES.contains("scala."));
+	}
+
+	/**
+	 * As long as we have Hadoop classes leaking through some of Flink's APIs (example bucketing sink),
+	 * we need to make them parent first.
+	 */
+	@Test
+	public void testHadoopParentFirst() {
+		assertTrue(PARENT_FIRST_PACKAGES.contains("org.apache.hadoop."));
+	}
+}


[3/3] flink git commit: [FLINK-8156] [build] Bump commons-beanutils version to 1.9.3

Posted by se...@apache.org.
[FLINK-8156] [build] Bump commons-beanutils version to 1.9.3

This closes #5113


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

Branch: refs/heads/master
Commit: 25f7aee13f022ee349cc96f7ac3b32760fa5017f
Parents: 1484080
Author: yew1eb <ye...@gmail.com>
Authored: Sun Dec 3 18:49:22 2017 +0800
Committer: Stephan Ewen <se...@apache.org>
Committed: Fri Jan 19 13:40:00 2018 +0100

----------------------------------------------------------------------
 flink-shaded-hadoop/flink-shaded-hadoop2/pom.xml | 3 ++-
 pom.xml                                          | 7 -------
 2 files changed, 2 insertions(+), 8 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/flink/blob/25f7aee1/flink-shaded-hadoop/flink-shaded-hadoop2/pom.xml
----------------------------------------------------------------------
diff --git a/flink-shaded-hadoop/flink-shaded-hadoop2/pom.xml b/flink-shaded-hadoop/flink-shaded-hadoop2/pom.xml
index efcbe07..f842c1c 100644
--- a/flink-shaded-hadoop/flink-shaded-hadoop2/pom.xml
+++ b/flink-shaded-hadoop/flink-shaded-hadoop2/pom.xml
@@ -171,7 +171,8 @@ under the License.
 		commons-collections dependency-->
 		<dependency>
 			<groupId>commons-beanutils</groupId>
-			<artifactId>commons-beanutils-bean-collections</artifactId>
+			<artifactId>commons-beanutils</artifactId>
+			<version>1.9.3</version>
 		</dependency>
 
 		<dependency>

http://git-wip-us.apache.org/repos/asf/flink/blob/25f7aee1/pom.xml
----------------------------------------------------------------------
diff --git a/pom.xml b/pom.xml
index 79e8b30..317b36a 100644
--- a/pom.xml
+++ b/pom.xml
@@ -372,13 +372,6 @@ under the License.
 				<version>3.2.2</version>
 			</dependency>
 
-			<!-- common-beanutils-bean-collections is used by flink-shaded-hadoop2 -->
-			<dependency>
-				<groupId>commons-beanutils</groupId>
-				<artifactId>commons-beanutils-bean-collections</artifactId>
-				<version>1.8.3</version>
-			</dependency>
-
 			<!--We have to bump the commons-configuration to version 1.7 because Hadoop uses per
 			default 1.6. This version has the problem that it depends on commons-beanutils-core and
 			commons-digester. Commons-digester depends on commons-beanutils. Both dependencies are


[2/3] flink git commit: [hotfix] [tests] Add unit tests for ChildFirstClassLoader

Posted by se...@apache.org.
[hotfix] [tests] Add unit tests for ChildFirstClassLoader


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

Branch: refs/heads/master
Commit: f9aff5926254ca05da77ea856e72bb2c71dae7fd
Parents: 25f7aee
Author: Stephan Ewen <se...@apache.org>
Authored: Thu Jan 18 17:58:01 2018 +0100
Committer: Stephan Ewen <se...@apache.org>
Committed: Fri Jan 19 13:40:00 2018 +0100

----------------------------------------------------------------------
 .../librarycache/FlinkUserCodeClassLoaders.java |  13 +-
 .../runtime/classloading/ClassLoaderTest.java   | 139 +++++++++++++++++++
 2 files changed, 146 insertions(+), 6 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/flink/blob/f9aff592/flink-runtime/src/main/java/org/apache/flink/runtime/execution/librarycache/FlinkUserCodeClassLoaders.java
----------------------------------------------------------------------
diff --git a/flink-runtime/src/main/java/org/apache/flink/runtime/execution/librarycache/FlinkUserCodeClassLoaders.java b/flink-runtime/src/main/java/org/apache/flink/runtime/execution/librarycache/FlinkUserCodeClassLoaders.java
index d40802e..59536a1 100644
--- a/flink-runtime/src/main/java/org/apache/flink/runtime/execution/librarycache/FlinkUserCodeClassLoaders.java
+++ b/flink-runtime/src/main/java/org/apache/flink/runtime/execution/librarycache/FlinkUserCodeClassLoaders.java
@@ -110,16 +110,17 @@ public class FlinkUserCodeClassLoaders {
 		protected synchronized Class<?> loadClass(
 			String name, boolean resolve) throws ClassNotFoundException {
 
-			for (String alwaysParentFirstPattern : alwaysParentFirstPatterns) {
-				if (name.startsWith(alwaysParentFirstPattern)) {
-					return super.loadClass(name, resolve);
-				}
-			}
-
 			// First, check if the class has already been loaded
 			Class<?> c = findLoadedClass(name);
 
 			if (c == null) {
+				// check whether the class should go parent-first
+				for (String alwaysParentFirstPattern : alwaysParentFirstPatterns) {
+					if (name.startsWith(alwaysParentFirstPattern)) {
+						return super.loadClass(name, resolve);
+					}
+				}
+
 				try {
 					// check the URLs
 					c = findClass(name);

http://git-wip-us.apache.org/repos/asf/flink/blob/f9aff592/flink-runtime/src/test/java/org/apache/flink/runtime/classloading/ClassLoaderTest.java
----------------------------------------------------------------------
diff --git a/flink-runtime/src/test/java/org/apache/flink/runtime/classloading/ClassLoaderTest.java b/flink-runtime/src/test/java/org/apache/flink/runtime/classloading/ClassLoaderTest.java
new file mode 100644
index 0000000..c02278c
--- /dev/null
+++ b/flink-runtime/src/test/java/org/apache/flink/runtime/classloading/ClassLoaderTest.java
@@ -0,0 +1,139 @@
+/*
+ * 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.runtime.classloading;
+
+import org.apache.flink.runtime.execution.librarycache.FlinkUserCodeClassLoaders;
+import org.apache.flink.util.TestLogger;
+
+import org.junit.Test;
+
+import java.net.URL;
+import java.net.URLClassLoader;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotEquals;
+
+/**
+ * Tests for classloading and class loder utilities.
+ */
+public class ClassLoaderTest extends TestLogger {
+
+	@Test
+	public void testParentFirstClassLoading() throws Exception {
+		final ClassLoader parentClassLoader = getClass().getClassLoader();
+
+		// collect the libraries / class folders with RocksDB related code: the state backend and RocksDB itself
+		final URL childCodePath = getClass().getProtectionDomain().getCodeSource().getLocation();
+
+		final URLClassLoader childClassLoader1 = FlinkUserCodeClassLoaders.parentFirst(
+				new URL[] { childCodePath }, parentClassLoader);
+
+		final URLClassLoader childClassLoader2 = FlinkUserCodeClassLoaders.parentFirst(
+				new URL[] { childCodePath }, parentClassLoader);
+
+		final String className = ClassLoaderTest.class.getName();
+
+		final Class<?> clazz1 = Class.forName(className, false, parentClassLoader);
+		final Class<?> clazz2 = Class.forName(className, false, childClassLoader1);
+		final Class<?> clazz3 = Class.forName(className, false, childClassLoader2);
+
+		assertEquals(clazz1, clazz2);
+		assertEquals(clazz1, clazz3);
+
+		childClassLoader1.close();
+		childClassLoader2.close();
+	}
+
+	@Test
+	public void testChildFirstClassLoading() throws Exception {
+		final ClassLoader parentClassLoader = getClass().getClassLoader();
+
+		// collect the libraries / class folders with RocksDB related code: the state backend and RocksDB itself
+		final URL childCodePath = getClass().getProtectionDomain().getCodeSource().getLocation();
+
+		final URLClassLoader childClassLoader1 = FlinkUserCodeClassLoaders.childFirst(
+				new URL[] { childCodePath }, parentClassLoader, new String[0]);
+
+		final URLClassLoader childClassLoader2 = FlinkUserCodeClassLoaders.childFirst(
+				new URL[] { childCodePath }, parentClassLoader, new String[0]);
+
+		final String className = ClassLoaderTest.class.getName();
+
+		final Class<?> clazz1 = Class.forName(className, false, parentClassLoader);
+		final Class<?> clazz2 = Class.forName(className, false, childClassLoader1);
+		final Class<?> clazz3 = Class.forName(className, false, childClassLoader2);
+
+		assertNotEquals(clazz1, clazz2);
+		assertNotEquals(clazz1, clazz3);
+		assertNotEquals(clazz2, clazz3);
+
+		childClassLoader1.close();
+		childClassLoader2.close();
+	}
+
+	@Test
+	public void testRepeatedChildFirstClassLoading() throws Exception {
+		final ClassLoader parentClassLoader = getClass().getClassLoader();
+
+		// collect the libraries / class folders with RocksDB related code: the state backend and RocksDB itself
+		final URL childCodePath = getClass().getProtectionDomain().getCodeSource().getLocation();
+
+		final URLClassLoader childClassLoader = FlinkUserCodeClassLoaders.childFirst(
+				new URL[] { childCodePath }, parentClassLoader, new String[0]);
+
+		final String className = ClassLoaderTest.class.getName();
+
+		final Class<?> clazz1 = Class.forName(className, false, parentClassLoader);
+		final Class<?> clazz2 = Class.forName(className, false, childClassLoader);
+		final Class<?> clazz3 = Class.forName(className, false, childClassLoader);
+		final Class<?> clazz4 = Class.forName(className, false, childClassLoader);
+
+		assertNotEquals(clazz1, clazz2);
+
+		assertEquals(clazz2, clazz3);
+		assertEquals(clazz2, clazz4);
+
+		childClassLoader.close();
+	}
+
+	@Test
+	public void testRepeatedParentFirstPatternClass() throws Exception {
+		final String className = ClassLoaderTest.class.getName();
+		final String parentFirstPattern = className.substring(0, className.lastIndexOf('.'));
+
+		final ClassLoader parentClassLoader = getClass().getClassLoader();
+
+		// collect the libraries / class folders with RocksDB related code: the state backend and RocksDB itself
+		final URL childCodePath = getClass().getProtectionDomain().getCodeSource().getLocation();
+
+		final URLClassLoader childClassLoader = FlinkUserCodeClassLoaders.childFirst(
+				new URL[] { childCodePath }, parentClassLoader, new String[] { parentFirstPattern });
+
+		final Class<?> clazz1 = Class.forName(className, false, parentClassLoader);
+		final Class<?> clazz2 = Class.forName(className, false, childClassLoader);
+		final Class<?> clazz3 = Class.forName(className, false, childClassLoader);
+		final Class<?> clazz4 = Class.forName(className, false, childClassLoader);
+
+		assertEquals(clazz1, clazz2);
+		assertEquals(clazz1, clazz3);
+		assertEquals(clazz1, clazz4);
+
+		childClassLoader.close();
+	}
+}