Index: hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestClassLoading.java =================================================================== --- hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestClassLoading.java (revision 1418070) +++ hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestClassLoading.java (working copy) @@ -210,16 +210,18 @@ new Path(fs.getUri().toString() + Path.SEPARATOR)); String jarFileOnHDFS1 = fs.getUri().toString() + Path.SEPARATOR + jarFile1.getName(); + Path pathOnHDFS1 = new Path(jarFileOnHDFS1); assertTrue("Copy jar file to HDFS failed.", - fs.exists(new Path(jarFileOnHDFS1))); + fs.exists(pathOnHDFS1)); LOG.info("Copied jar file to HDFS: " + jarFileOnHDFS1); fs.copyFromLocalFile(new Path(jarFile2.getPath()), new Path(fs.getUri().toString() + Path.SEPARATOR)); String jarFileOnHDFS2 = fs.getUri().toString() + Path.SEPARATOR + jarFile2.getName(); + Path pathOnHDFS2 = new Path(jarFileOnHDFS2); assertTrue("Copy jar file to HDFS failed.", - fs.exists(new Path(jarFileOnHDFS2))); + fs.exists(pathOnHDFS2)); LOG.info("Copied jar file to HDFS: " + jarFileOnHDFS2); // create a table that references the coprocessors @@ -238,7 +240,10 @@ } admin.deleteTable(tableName); } - admin.createTable(htd); + CoprocessorHost.clearCacheForTesting(); + byte[] startKey = {10, 63}; + byte[] endKey = {12, 43}; + admin.createTable(htd, startKey, endKey, 4); waitForTable(htd.getName()); // verify that the coprocessors were loaded @@ -268,6 +273,10 @@ assertTrue("Configuration key 'k1' was missing on a region", found2_k1); assertTrue("Configuration key 'k2' was missing on a region", found2_k2); assertTrue("Configuration key 'k3' was missing on a region", found2_k3); + assertTrue(jarFileOnHDFS1 + " should have been loaded once", + CoprocessorHost.getClassloaderCountForTesting(pathOnHDFS1) == 1); + assertTrue(jarFileOnHDFS2 + " should have been loaded once", + CoprocessorHost.getClassloaderCountForTesting(pathOnHDFS2) == 1); } @Test Index: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionCoprocessorHost.java =================================================================== --- hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionCoprocessorHost.java (revision 1418070) +++ hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionCoprocessorHost.java (working copy) @@ -342,6 +342,7 @@ } shutdown(env); } + activeCoprocessorClassLoaders.clear(); } /** Index: hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/CoprocessorHost.java =================================================================== --- hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/CoprocessorHost.java (revision 1418070) +++ hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/CoprocessorHost.java (working copy) @@ -19,6 +19,7 @@ package org.apache.hadoop.hbase.coprocessor; +import com.google.common.collect.MapMaker; import com.google.protobuf.Service; import com.google.protobuf.ServiceException; import org.apache.commons.logging.Log; @@ -48,6 +49,7 @@ import java.io.IOException; import java.net.URL; import java.util.*; +import java.util.concurrent.ConcurrentMap; import java.util.jar.JarEntry; import java.util.jar.JarFile; @@ -78,6 +80,11 @@ protected String pathPrefix; protected volatile int loadSequence; + // classloader caching + protected Set activeCoprocessorClassLoaders = new HashSet(); + static ConcurrentMap classLoadersCache = new MapMaker() + .concurrencyLevel(3).weakValues().makeMap(); + public CoprocessorHost() { pathPrefix = UUID.randomUUID().toString(); } @@ -150,6 +157,19 @@ coprocessors.addAll(configured); } + // clears classLoadersCache. Used by unit tests + static void clearCacheForTesting() { + classLoadersCache.clear(); + } + /* + * @param path + * @return the number of class loaders corresponding to path + */ + static int getClassloaderCountForTesting(Path path) { + ClassLoader cl = classLoadersCache.get(path); + return cl == null ? 0 : 1; + } + /** * Load a coprocessor implementation into the host * @param path path to implementation jar @@ -165,14 +185,27 @@ LOG.debug("Loading coprocessor class " + className + " with path " + path + " and priority " + priority); - // Have we already loaded the class, perhaps from an earlier region open - // for the same table? - try { - implClass = getClass().getClassLoader().loadClass(className); - } catch (ClassNotFoundException e) { - LOG.info("Class " + className + " needs to be loaded from a file - " + - path + "."); - // go ahead to load from file system. + ClassLoader cl = null; + if (path == null) { + try { + implClass = getClass().getClassLoader().loadClass(className); + } catch (ClassNotFoundException e) { + throw new IOException("No jar path specified for " + className); + } + } else { + // Have we already loaded the class, perhaps from an earlier region open + // for the same table? + cl = classLoadersCache.get(path); + if (cl != null){ + LOG.debug("Found classloader "+ cl + "for "+path.toString()); + try { + implClass = cl.loadClass(className); + } catch (ClassNotFoundException e) { + LOG.info("Class " + className + " needs to be loaded from a file - " + + path + "."); + // go ahead to load from file system. + } + } } // If not, load @@ -204,7 +237,8 @@ // unsurprisingly wants URLs, not URIs; so we will use the deprecated // method which returns URLs for as long as it is available List paths = new ArrayList(); - paths.add(new File(dst.toString()).getCanonicalFile().toURL()); + URL url = new File(dst.toString()).getCanonicalFile().toURL(); + paths.add(url); JarFile jarFile = new JarFile(dst.toString()); Enumeration entries = jarFile.entries(); @@ -221,17 +255,38 @@ } jarFile.close(); - ClassLoader cl = new CoprocessorClassLoader(paths, - this.getClass().getClassLoader()); - Thread.currentThread().setContextClassLoader(cl); + cl = new CoprocessorClassLoader(paths, this.getClass().getClassLoader()); + ClassLoader prev = classLoadersCache.putIfAbsent(path, cl); + if (prev != null) { + //lost update race, use already added classloader + cl = prev; + } + try { implClass = cl.loadClass(className); + // cache cp classloader as a weak value, will be GC'ed when no reference left + classLoadersCache.put (path, cl); + // keep a reference to cached classloader at this instance level + // when no CPH refer the weak value of classloader it will be GC'ed + activeCoprocessorClassLoaders.add(cl); } catch (ClassNotFoundException e) { + classLoadersCache.remove(path); throw new IOException(e); } + } - return loadInstance(implClass, priority, conf); + //load custom code for coprocessor + Thread currentThread = Thread.currentThread(); + ClassLoader hostClassLoader = currentThread.getContextClassLoader(); + try{ + // switch temporarily to the thread classloader for custom CP + currentThread.setContextClassLoader(cl); + return loadInstance(implClass, priority, conf); + } finally { + // restore the fresh (host) classloader + currentThread.setContextClassLoader(hostClassLoader); + } } /** Index: hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/CoprocessorClassLoader.java =================================================================== --- hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/CoprocessorClassLoader.java (revision 1418070) +++ hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/CoprocessorClassLoader.java (working copy) @@ -65,9 +65,13 @@ "org.w3c", "org.xml", "sunw.", - // Hadoop/HBase: + // logging + "org.apache.commons.logging", + "org.apache.log4j", + "com.hadoop", + // Hadoop/HBase/ZK: "org.apache.hadoop", - "com.hadoop", + "org.apache.zookeeper", }; /** @@ -82,14 +86,23 @@ }; /** + * Parent classloader used to load any class not matching the exemption list. + */ + private ClassLoader parent; + + /** * Creates a CoprocessorClassLoader that loads classes from the given paths. * @param paths paths from which to load classes. * @param parent the parent ClassLoader to set. */ public CoprocessorClassLoader(List paths, ClassLoader parent) { super(paths.toArray(new URL[]{}), parent); + this.parent = parent; + if (parent == null) { + throw new IllegalArgumentException("No parent classloader!"); + } } - + @Override synchronized public Class loadClass(String name) throws ClassNotFoundException { @@ -99,9 +112,9 @@ LOG.debug("Skipping exempt class " + name + " - delegating directly to parent"); } - return super.loadClass(name); + return parent.loadClass(name); } - + // Check whether the class has already been loaded: Class clasz = findLoadedClass(name); if (clasz != null) { @@ -123,7 +136,7 @@ LOG.debug("Class " + name + " not found - delegating to parent"); } try { - clasz = super.loadClass(name); + clasz = parent.loadClass(name); } catch (ClassNotFoundException e2) { // Class not found in this ClassLoader or in the parent ClassLoader // Log some debug output before rethrowing ClassNotFoundException