You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tez.apache.org by ha...@apache.org on 2020/08/13 06:30:07 UTC

[tez] 02/02: TEZ-4223 - Adding new jars or resources after the first DAG runs does not work.

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 <ha...@apache.org>
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 <rb...@apache.org>
---
 .../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 =