This is an automated email from the ASF dual-hosted git repository. harishjp pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/tez.git
commit c047fde127a7ec2448c7851e89366cb3b0b03136 Author: Harish JP <[email protected]> AuthorDate: Thu Aug 13 11:59:40 2020 +0530 TEZ-4223 - Adding new jars or resources after the first DAG runs does not work. Signed-off-by: Rajesh Balamohan <[email protected]> --- .../org/apache/tez/common/ReflectionUtils.java | 43 +-------------------- .../java/org/apache/tez/common/TezClassLoader.java | 37 ++++++++---------- .../org/apache/tez/client/TestTezClientUtils.java | 44 ++++++++++++++++------ .../org/apache/tez/common/TestReflectionUtils.java | 4 +- .../java/org/apache/tez/dag/app/DAGAppMaster.java | 2 + 5 files changed, 55 insertions(+), 75 deletions(-) diff --git a/tez-api/src/main/java/org/apache/tez/common/ReflectionUtils.java b/tez-api/src/main/java/org/apache/tez/common/ReflectionUtils.java index 9f7c5d3..73becda 100644 --- a/tez-api/src/main/java/org/apache/tez/common/ReflectionUtils.java +++ b/tez-api/src/main/java/org/apache/tez/common/ReflectionUtils.java @@ -19,17 +19,14 @@ package org.apache.tez.common; import java.lang.reflect.Constructor; -import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; import java.net.URL; -import java.net.URLClassLoader; import java.util.List; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; import org.apache.hadoop.classification.InterfaceAudience.Private; import org.apache.tez.dag.api.TezReflectionException; -import org.apache.tez.dag.api.TezUncheckedException; @Private public class ReflectionUtils { @@ -110,46 +107,10 @@ public class ReflectionUtils { } @Private - public static synchronized void addResourcesToClasspath(List<URL> urls) { - ClassLoader classLoader = new URLClassLoader(urls.toArray(new URL[urls.size()]), Thread - .currentThread().getContextClassLoader()); - Thread.currentThread().setContextClassLoader(classLoader); - } - - // Parameters for addResourcesToSystemClassLoader - private static final Class<?>[] parameters = new Class[]{URL.class}; - private static Method sysClassLoaderMethod = null; - - @Private public static synchronized void addResourcesToSystemClassLoader(List<URL> urls) { - ClassLoader sysLoader = getSystemClassLoader(); - if (sysClassLoaderMethod == null) { - Class<?> sysClass = TezClassLoader.class; - Method method; - try { - method = sysClass.getDeclaredMethod("addURL", parameters); - } catch (SecurityException e) { - throw new TezUncheckedException("Failed to get handle on method addURL", e); - } catch (NoSuchMethodException e) { - throw new TezUncheckedException("Failed to get handle on method addURL", e); - } - method.setAccessible(true); - sysClassLoaderMethod = method; - } + TezClassLoader classLoader = TezClassLoader.getInstance(); for (URL url : urls) { - try { - sysClassLoaderMethod.invoke(sysLoader, new Object[] { url }); - } catch (IllegalArgumentException e) { - throw new TezUncheckedException("Failed to invoke addURL for rsrc: " + url, e); - } catch (IllegalAccessException e) { - throw new TezUncheckedException("Failed to invoke addURL for rsrc: " + url, e); - } catch (InvocationTargetException e) { - throw new TezUncheckedException("Failed to invoke addURL for rsrc: " + url, e); - } + classLoader.addURL(url); } } - - private static ClassLoader getSystemClassLoader() { - return TezClassLoader.getInstance(); - } } diff --git a/tez-api/src/main/java/org/apache/tez/common/TezClassLoader.java b/tez-api/src/main/java/org/apache/tez/common/TezClassLoader.java index 2679efa..923d217 100644 --- a/tez-api/src/main/java/org/apache/tez/common/TezClassLoader.java +++ b/tez-api/src/main/java/org/apache/tez/common/TezClassLoader.java @@ -13,30 +13,34 @@ */ package org.apache.tez.common; -import java.net.MalformedURLException; import java.net.URL; import java.net.URLClassLoader; import java.security.AccessController; import java.security.PrivilegedAction; -import java.util.Arrays; +/** + * ClassLoader to allow addition of new paths to classpath in the runtime. + * + * It uses URLClassLoader with this class' classloader as parent classloader. + * And hence first delegates the resource loading to parent and then to the URLs + * added. The process must be setup to use by invoking setupTezClassLoader() which sets + * the global TezClassLoader as current thread context class loader. All threads + * created will inherit the classloader and hence will resolve the class/resource + * from TezClassLoader. + */ public class TezClassLoader extends URLClassLoader { - private static TezClassLoader INSTANCE; + private static final TezClassLoader INSTANCE; static { INSTANCE = AccessController.doPrivileged(new PrivilegedAction<TezClassLoader>() { - ClassLoader sysLoader = TezClassLoader.class.getClassLoader(); - public TezClassLoader run() { - return new TezClassLoader( - sysLoader instanceof URLClassLoader ? ((URLClassLoader) sysLoader).getURLs() : extractClassPathEntries(), - sysLoader); + return new TezClassLoader(); } }); } - public TezClassLoader(URL[] urls, ClassLoader classLoader) { - super(urls, classLoader); + private TezClassLoader() { + super(new URL[] {}, TezClassLoader.class.getClassLoader()); } public void addURL(URL url) { @@ -47,16 +51,7 @@ public class TezClassLoader extends URLClassLoader { return INSTANCE; } - private static URL[] extractClassPathEntries() { - String pathSeparator = System.getProperty("path.separator"); - String[] classPathEntries = System.getProperty("java.class.path").split(pathSeparator); - URL[] cp = Arrays.asList(classPathEntries).stream().map(s -> { - try { - return new URL("file://" + s); - } catch (MalformedURLException e) { - throw new RuntimeException(e); - } - }).toArray(URL[]::new); - return cp; + public static void setupTezClassLoader() { + Thread.currentThread().setContextClassLoader(INSTANCE); } } diff --git a/tez-api/src/test/java/org/apache/tez/client/TestTezClientUtils.java b/tez-api/src/test/java/org/apache/tez/client/TestTezClientUtils.java index adcc65c..29e9210 100644 --- a/tez-api/src/test/java/org/apache/tez/client/TestTezClientUtils.java +++ b/tez-api/src/test/java/org/apache/tez/client/TestTezClientUtils.java @@ -26,6 +26,7 @@ import static org.junit.Assert.assertTrue; import java.io.File; import java.io.FileNotFoundException; import java.io.IOException; +import java.net.MalformedURLException; import java.net.URL; import java.nio.ByteBuffer; import java.util.ArrayList; @@ -61,7 +62,6 @@ import org.apache.hadoop.yarn.api.records.Resource; import org.apache.hadoop.yarn.conf.YarnConfiguration; import org.apache.hadoop.yarn.exceptions.YarnException; import org.apache.hadoop.yarn.util.Records; -import org.apache.tez.common.TezClassLoader; import org.apache.tez.common.security.JobTokenIdentifier; import org.apache.tez.common.security.JobTokenSecretManager; import org.apache.tez.common.security.TokenCache; @@ -83,7 +83,7 @@ import org.junit.Test; public class TestTezClientUtils { private static String TEST_ROOT_DIR = "target" + Path.SEPARATOR + TestTezClientUtils.class.getName() + "-tmpDir"; - private static final File STAGING_DIR = new File(System.getProperty("test.build.data"), + private static final File STAGING_DIR = new File(System.getProperty("test.build.data", "target"), TestTezClientUtils.class.getName()).getAbsoluteFile(); /** * @@ -132,12 +132,29 @@ public class TestTezClientUtils { TezClientUtils.setupTezJarsLocalResources(conf, credentials, resources); } - /** - * - */ + private static List<URL> getDirAndFileURL() throws MalformedURLException { + String[] classpaths = System.getProperty("java.class.path") + .split(System.getProperty("path.separator")); + List<URL> urls = new ArrayList<>(2); + File lastFile = null; + // Add one file and one directory. + for (String path : classpaths) { + URL url = new URL("file://" + path); + File file = FileUtils.toFile(url); + if (lastFile == null) { + lastFile = file; + urls.add(url); + } else if (lastFile.isDirectory() != file.isDirectory()) { + urls.add(url); + break; + } + } + return urls; + } + @Test (timeout=20000) public void validateSetTezJarLocalResourcesDefinedExistingDirectory() throws Exception { - URL[] cp = TezClassLoader.getInstance().getURLs(); + List<URL> cp = getDirAndFileURL(); StringBuffer buffer = new StringBuffer(); for (URL url : cp) { buffer.append(url.toExternalForm()); @@ -151,22 +168,27 @@ public class TestTezClientUtils { localizedMap); Assert.assertFalse(usingArchive); Set<String> resourceNames = localizedMap.keySet(); + boolean assertedDir = false; + boolean assertedFile = false; for (URL url : cp) { File file = FileUtils.toFile(url); - if (file.isDirectory()){ + if (file.isDirectory()) { String[] firList = file.list(); for (String fileNme : firList) { File innerFile = new File(file, fileNme); if (!innerFile.isDirectory()){ assertTrue(resourceNames.contains(innerFile.getName())); + assertedDir = true; } // not supporting deep hierarchies } - } - else { + } else { assertTrue(resourceNames.contains(file.getName())); + assertedFile = true; } } + assertTrue(assertedDir); + assertTrue(assertedFile); } /** @@ -175,7 +197,7 @@ public class TestTezClientUtils { */ @Test (timeout=5000) public void validateSetTezJarLocalResourcesDefinedExistingDirectoryIgnored() throws Exception { - URL[] cp = TezClassLoader.getInstance().getURLs(); + List<URL> cp = getDirAndFileURL(); StringBuffer buffer = new StringBuffer(); for (URL url : cp) { buffer.append(url.toExternalForm()); @@ -196,7 +218,7 @@ public class TestTezClientUtils { */ @Test (timeout=20000) public void validateSetTezJarLocalResourcesDefinedExistingDirectoryIgnoredSetToFalse() throws Exception { - URL[] cp = TezClassLoader.getInstance().getURLs(); + List<URL> cp = getDirAndFileURL(); StringBuffer buffer = new StringBuffer(); for (URL url : cp) { buffer.append(url.toExternalForm()); diff --git a/tez-api/src/test/java/org/apache/tez/common/TestReflectionUtils.java b/tez-api/src/test/java/org/apache/tez/common/TestReflectionUtils.java index 2fbd35c..ed3814d 100644 --- a/tez-api/src/test/java/org/apache/tez/common/TestReflectionUtils.java +++ b/tez-api/src/test/java/org/apache/tez/common/TestReflectionUtils.java @@ -58,7 +58,7 @@ public class TestReflectionUtils { @Test(timeout = 5000) public void testAddResourceToClasspath() throws IOException, TezException { - + TezClassLoader.setupTezClassLoader(); String rsrcName = "dummyfile.xml"; FileSystem localFs = FileSystem.getLocal(new Configuration()); Path p = new Path(rsrcName); @@ -78,7 +78,7 @@ public class TestReflectionUtils { urlForm = urlForm.substring(0, urlForm.lastIndexOf('/') + 1); URL url = new URL(urlForm); - ReflectionUtils.addResourcesToClasspath(Collections.singletonList(url)); + ReflectionUtils.addResourcesToSystemClassLoader(Collections.singletonList(url)); loadedUrl = Thread.currentThread().getContextClassLoader().getResource(rsrcName); diff --git a/tez-dag/src/main/java/org/apache/tez/dag/app/DAGAppMaster.java b/tez-dag/src/main/java/org/apache/tez/dag/app/DAGAppMaster.java index 7e5a7a9..fcfb883 100644 --- a/tez-dag/src/main/java/org/apache/tez/dag/app/DAGAppMaster.java +++ b/tez-dag/src/main/java/org/apache/tez/dag/app/DAGAppMaster.java @@ -2328,6 +2328,8 @@ public class DAGAppMaster extends AbstractService { public static void main(String[] args) { try { + // Install the tez class loader, which can be used add new resources + TezClassLoader.setupTezClassLoader(); Thread.setDefaultUncaughtExceptionHandler(new YarnUncaughtExceptionHandler()); final String pid = System.getenv().get("JVM_PID"); String containerIdStr =
