You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by ta...@apache.org on 2018/10/23 22:55:36 UTC
[2/2] impala git commit: IMPALA-7668: Proper clean up of
URLClassLoader
IMPALA-7668: Proper clean up of URLClassLoader
Starting with Java 7 URLClassLoader implements a close() method that
cleans up any open files and helps avoid bugs like fd leaks inside the
class loader. Additionally it also helps load updated class versions
of the classes that are loaded already by prior instances.
This commit makes sure that the URLClassLoader is close()'d in a few
places in the code.
Testing: Tricky to automate the tests for this behavior, so no new
tests were added.
Change-Id: I5c5100ef5c5a97d92d94fb68daab622f0aa30158
Reviewed-on: http://gerrit.cloudera.org:8080/11594
Reviewed-by: Bharath Vissapragada <bh...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
Project: http://git-wip-us.apache.org/repos/asf/impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/impala/commit/e0c54b7c
Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/e0c54b7c
Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/e0c54b7c
Branch: refs/heads/master
Commit: e0c54b7c8ee05a88cec6f4f7a479971c45271bbb
Parents: 0e80b4b
Author: Bharath Vissapragada <bh...@cloudera.com>
Authored: Thu Oct 4 23:20:30 2018 -0700
Committer: Impala Public Jenkins <im...@cloudera.com>
Committed: Tue Oct 23 21:50:16 2018 +0000
----------------------------------------------------------------------
.../ExternalDataSourceExecutor.java | 4 +-
.../impala/hive/executor/UdfExecutor.java | 14 ++++-
.../org/apache/impala/util/FunctionUtils.java | 56 ++++++++++----------
3 files changed, 43 insertions(+), 31 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/impala/blob/e0c54b7c/fe/src/main/java/org/apache/impala/extdatasource/ExternalDataSourceExecutor.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/extdatasource/ExternalDataSourceExecutor.java b/fe/src/main/java/org/apache/impala/extdatasource/ExternalDataSourceExecutor.java
index a7624d6..cd0644b 100644
--- a/fe/src/main/java/org/apache/impala/extdatasource/ExternalDataSourceExecutor.java
+++ b/fe/src/main/java/org/apache/impala/extdatasource/ExternalDataSourceExecutor.java
@@ -145,8 +145,8 @@ public class ExternalDataSourceExecutor {
c = cachedClasses_.get(cacheMapKey);
if (c == null) {
URL url = new File(jarPath_).toURI().toURL();
- URLClassLoader loader = URLClassLoader.newInstance(
- new URL[] { url }, getClass().getClassLoader());
+ URLClassLoader loader = URLClassLoader.newInstance( new URL[] { url },
+ getClass().getClassLoader());
c = Class.forName(className_, true, loader);
if (!ArrayUtils.contains(c.getInterfaces(), apiVersion_.getApiInterface())) {
throw new ImpalaRuntimeException(String.format(
http://git-wip-us.apache.org/repos/asf/impala/blob/e0c54b7c/fe/src/main/java/org/apache/impala/hive/executor/UdfExecutor.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/hive/executor/UdfExecutor.java b/fe/src/main/java/org/apache/impala/hive/executor/UdfExecutor.java
index cfe101b..64af76c 100644
--- a/fe/src/main/java/org/apache/impala/hive/executor/UdfExecutor.java
+++ b/fe/src/main/java/org/apache/impala/hive/executor/UdfExecutor.java
@@ -18,6 +18,7 @@
package org.apache.impala.hive.executor;
import java.io.File;
+import java.io.IOException;
import java.lang.reflect.Constructor;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
@@ -577,9 +578,10 @@ public class UdfExecutor {
Type retType, Type... parameterTypes) throws
ImpalaRuntimeException {
ArrayList<String> signatures = Lists.newArrayList();
+ ClassLoader loader = null;
try {
LOG.debug("Loading UDF '" + udfPath + "' from " + jarPath);
- ClassLoader loader = getClassLoader(jarPath);
+ loader = getClassLoader(jarPath);
Class<?> c = Class.forName(udfPath, true, loader);
Class<? extends UDF> udfClass = c.asSubclass(UDF.class);
Constructor<? extends UDF> ctor = udfClass.getConstructor();
@@ -637,6 +639,16 @@ public class UdfExecutor {
throw new ImpalaRuntimeException("Unable to call create UDF instance.", e);
} catch (InvocationTargetException e) {
throw new ImpalaRuntimeException("Unable to call create UDF instance.", e);
+ } finally {
+ // Clean up
+ if (jarPath != null && loader instanceof URLClassLoader) {
+ try {
+ ((URLClassLoader)loader).close();
+ } catch (IOException e) {
+ // Log and ignore.
+ LOG.debug("Error closing the URLClassloader.", e);
+ }
+ }
}
}
}
http://git-wip-us.apache.org/repos/asf/impala/blob/e0c54b7c/fe/src/main/java/org/apache/impala/util/FunctionUtils.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/util/FunctionUtils.java b/fe/src/main/java/org/apache/impala/util/FunctionUtils.java
index 686bf18..8b7a5f8 100644
--- a/fe/src/main/java/org/apache/impala/util/FunctionUtils.java
+++ b/fe/src/main/java/org/apache/impala/util/FunctionUtils.java
@@ -95,34 +95,34 @@ public abstract class FunctionUtils {
throw new ImpalaRuntimeException(errorMsg);
}
URL[] classLoaderUrls = new URL[] {new URL(localJarPath.toString())};
- URLClassLoader urlClassLoader = new URLClassLoader(classLoaderUrls);
- // TODO(todd): above class loader is leaked without closing.
- udfClass = urlClassLoader.loadClass(function.getClassName());
- // Check if the class is of UDF type. Currently we don't support other functions
- // TODO: Remove this once we support Java UDAF/UDTF
- if (org.apache.hadoop.hive.ql.exec.FunctionUtils.getUDFClassType(udfClass) !=
- org.apache.hadoop.hive.ql.exec.FunctionUtils.UDFClassType.UDF) {
- LOG.warn("Ignoring load of incompatible Java function: " +
- function.getFunctionName() + " as " +
- org.apache.hadoop.hive.ql.exec.FunctionUtils.getUDFClassType(udfClass)
- + " is not a supported type. Only UDFs are supported");
- return result;
- }
- // Load each method in the UDF class and create the corresponding Impala Function
- // object.
- for (Method m: udfClass.getMethods()) {
- if (!m.getName().equals(UdfExecutor.UDF_FUNCTION_NAME)) continue;
- Function fn = ScalarFunction.fromHiveFunction(db,
- function.getFunctionName(), function.getClassName(),
- m.getParameterTypes(), m.getReturnType(), jarUri);
- if (fn == null) {
- LOG.warn("Ignoring incompatible method: " + m.toString() + " during load of " +
- "Hive UDF:" + function.getFunctionName() + " from " + udfClass);
- continue;
- }
- if (!addedSignatures.contains(fn.signatureString())) {
- result.add(fn);
- addedSignatures.add(fn.signatureString());
+ try (URLClassLoader urlClassLoader = new URLClassLoader(classLoaderUrls)) {
+ udfClass = urlClassLoader.loadClass(function.getClassName());
+ // Check if the class is of UDF type. Currently we don't support other functions
+ // TODO: Remove this once we support Java UDAF/UDTF
+ if (org.apache.hadoop.hive.ql.exec.FunctionUtils.getUDFClassType(udfClass) !=
+ org.apache.hadoop.hive.ql.exec.FunctionUtils.UDFClassType.UDF) {
+ LOG.warn("Ignoring load of incompatible Java function: " +
+ function.getFunctionName() + " as " +
+ org.apache.hadoop.hive.ql.exec.FunctionUtils.getUDFClassType(udfClass)
+ + " is not a supported type. Only UDFs are supported");
+ return result;
+ }
+ // Load each method in the UDF class and create the corresponding Impala Function
+ // object.
+ for (Method m: udfClass.getMethods()) {
+ if (!m.getName().equals(UdfExecutor.UDF_FUNCTION_NAME)) continue;
+ Function fn = ScalarFunction.fromHiveFunction(db,
+ function.getFunctionName(), function.getClassName(),
+ m.getParameterTypes(), m.getReturnType(), jarUri);
+ if (fn == null) {
+ LOG.warn("Ignoring incompatible method: " + m.toString() + " during load of "
+ + "Hive UDF:" + function.getFunctionName() + " from " + udfClass);
+ continue;
+ }
+ if (!addedSignatures.contains(fn.signatureString())) {
+ result.add(fn);
+ addedSignatures.add(fn.signatureString());
+ }
}
}
} catch (ClassNotFoundException c) {