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:05 UTC

[tez] branch master updated (6fc75ad -> c047fde)

This is an automated email from the ASF dual-hosted git repository.

harishjp pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/tez.git.


    from 6fc75ad  TEZ-4223 - Adding new jars or resources after the first DAG runs does not work.
     new 59419dd  Revert "TEZ-4223 - Adding new jars or resources after the first DAG runs does not work."
     new c047fde  TEZ-4223 - Adding new jars or resources after the first DAG runs does not work.

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../java/org/apache/tez/common/TezClassLoader.java | 12 +++++-
 .../org/apache/tez/client/TestTezClientUtils.java  | 44 ++++++++++++++++------
 2 files changed, 44 insertions(+), 12 deletions(-)


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

Posted by ha...@apache.org.
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 59419dd3e3817d52f3a7ace34f122c92bea03d33
Author: Harish JP <ha...@apache.org>
AuthorDate: Thu Aug 13 11:59:03 2020 +0530

    Revert "TEZ-4223 - Adding new jars or resources after the first DAG runs does not work."
    
    This reverts commit 6fc75ad6e9b1601b8b14dd85fa9b0aea53585fba.
---
 .../org/apache/tez/common/ReflectionUtils.java     | 43 ++++++++++++++++++++-
 .../java/org/apache/tez/common/TezClassLoader.java | 45 ++++++++++++++--------
 .../org/apache/tez/common/TestReflectionUtils.java |  4 +-
 .../java/org/apache/tez/dag/app/DAGAppMaster.java  |  2 -
 4 files changed, 73 insertions(+), 21 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 73becda..9f7c5d3 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,14 +19,17 @@
 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 {
@@ -107,10 +110,46 @@ 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) {
-    TezClassLoader classLoader = TezClassLoader.getInstance();
+    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;
+    }
     for (URL url : urls) {
-      classLoader.addURL(url);
+      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);
+      }
     }
   }
+
+  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 1842a19..2679efa 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,24 +13,30 @@
  */
 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 = new TezClassLoader();
+  private static 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);
+      }
+    });
+  }
 
-  private TezClassLoader() {
-    super(new URL[] {}, TezClassLoader.class.getClassLoader());
+  public TezClassLoader(URL[] urls, ClassLoader classLoader) {
+    super(urls, classLoader);
   }
 
   public void addURL(URL url) {
@@ -41,7 +47,16 @@ public class TezClassLoader extends URLClassLoader {
     return INSTANCE;
   }
 
-  public static void setupTezClassLoader() {
-    Thread.currentThread().setContextClassLoader(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;
   }
 }
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 ed3814d..2fbd35c 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.addResourcesToSystemClassLoader(Collections.singletonList(url));
+      ReflectionUtils.addResourcesToClasspath(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 fcfb883..7e5a7a9 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,8 +2328,6 @@ 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 =


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

Posted by ha...@apache.org.
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 =