Repository: lens Updated Branches: refs/heads/master 1362e667e -> df75a4b83
LENS-1160: Fix classloader getting closed on lens session close. Project: http://git-wip-us.apache.org/repos/asf/lens/repo Commit: http://git-wip-us.apache.org/repos/asf/lens/commit/df75a4b8 Tree: http://git-wip-us.apache.org/repos/asf/lens/tree/df75a4b8 Diff: http://git-wip-us.apache.org/repos/asf/lens/diff/df75a4b8 Branch: refs/heads/master Commit: df75a4b838f5572ff6ef2c7466681853ac00ecb4 Parents: 1362e66 Author: Rajat Khandelwal <pro...@apache.org> Authored: Wed Jun 8 23:11:40 2016 +0530 Committer: Rajat Khandelwal <rajatgupt...@gmail.com> Committed: Wed Jun 8 23:11:40 2016 +0530 ---------------------------------------------------------------------- .../server/session/DatabaseResourceService.java | 96 ++-------- .../lens/server/session/LensSessionImpl.java | 148 ++++++++++----- .../lens/server/session/SessionClassLoader.java | 55 ++++++ .../server/session/UncloseableClassLoader.java | 41 +++++ .../server/session/TestSessionClassLoaders.java | 180 ++++++++++++++++--- 5 files changed, 371 insertions(+), 149 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/lens/blob/df75a4b8/lens-server/src/main/java/org/apache/lens/server/session/DatabaseResourceService.java ---------------------------------------------------------------------- diff --git a/lens-server/src/main/java/org/apache/lens/server/session/DatabaseResourceService.java b/lens-server/src/main/java/org/apache/lens/server/session/DatabaseResourceService.java index 1f63ed7..511e4cf 100644 --- a/lens-server/src/main/java/org/apache/lens/server/session/DatabaseResourceService.java +++ b/lens-server/src/main/java/org/apache/lens/server/session/DatabaseResourceService.java @@ -21,7 +21,6 @@ package org.apache.lens.server.session; import java.io.IOException; import java.net.MalformedURLException; import java.net.URL; -import java.net.URLClassLoader; import java.util.*; import org.apache.lens.server.LensServices; @@ -34,7 +33,6 @@ import org.apache.commons.lang3.StringUtils; import org.apache.hadoop.fs.FileStatus; import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.Path; -import org.apache.hadoop.hive.conf.HiveConf; import org.apache.hive.service.AbstractService; import lombok.extern.slf4j.Slf4j; @@ -45,9 +43,8 @@ import lombok.extern.slf4j.Slf4j; @Slf4j public class DatabaseResourceService extends AbstractService { public static final String NAME = "database-resources"; - private Map<String, ClassLoader> classLoaderCache; - private Map<String, List<LensSessionImpl.ResourceEntry>> dbResEntryMap; - + private Map<String, UncloseableClassLoader> classLoaderCache = new HashMap<>(); + private final Map<String, List<LensSessionImpl.ResourceEntry>> dbResEntryMap = new HashMap<>(); /** * The metrics service. */ @@ -73,16 +70,8 @@ public class DatabaseResourceService extends AbstractService { } @Override - public synchronized void init(HiveConf hiveConf) { - super.init(hiveConf); - classLoaderCache = new HashMap<>(); - dbResEntryMap = new HashMap<>(); - } - - @Override public synchronized void start() { super.start(); - try { log.info("Starting loading DB specific resources"); loadDbResourceEntries(); @@ -197,90 +186,34 @@ public class DatabaseResourceService extends AbstractService { /** * Load DB specific resources */ - public void loadResources() { + private void loadResources() { for (String db : dbResEntryMap.keySet()) { - try { - createClassLoader(db); - loadDBJars(db, dbResEntryMap.get(db), true); - log.info("Loaded resources for db {} resources: {}", db, dbResEntryMap.get(db)); - } catch (LensException exc) { - incrCounter(LOAD_RESOURCES_ERRORS); - log.warn("Failed to load resources for db {}", db, exc); - classLoaderCache.remove(db); - } + loadDBJars(db, dbResEntryMap.get(db)); + log.info("Loaded resources for db {} resources: {}", db, dbResEntryMap.get(db)); } } - protected void createClassLoader(String database) throws LensException { - classLoaderCache.put(database, this.getClass().getClassLoader()); - } - /** * Add a resource to the specified database. Update class loader of the database if required. * @param database database name * @param resources resources which need to be added to the database - * @param useUri if set to true, use URI from resourceEntry to load in classLoader and update class loader of the - * database in the class loader cache - * @return class loader updated as a result of adding any JARs */ - private synchronized ClassLoader loadDBJars(String database, Collection<LensSessionImpl.ResourceEntry> resources, - boolean useUri) { - ClassLoader classLoader = classLoaderCache.get(database); - if (classLoader == null) { - // No change since there are no static resources to be added - return null; - } - - if (resources == null || resources.isEmpty()) { - // Return DB class loader directly since no resources have to be merged. - return classLoader; - } - - // Get URLs of the class loader - if (classLoader instanceof URLClassLoader) { - URLClassLoader urlLoader = (URLClassLoader) classLoader; - URL[] preUrls = urlLoader.getURLs(); - - // Add to set to remove duplicate additions - Set<URL> newUrls = new LinkedHashSet<>(); - // New class loader = URLs of DB jars + argument jars - Collections.addAll(newUrls, preUrls); - + private synchronized void loadDBJars(String database, Collection<LensSessionImpl.ResourceEntry> resources) { + URL[] urls = new URL[0]; + if (resources != null) { + urls = new URL[resources.size()]; + int i = 0; for (LensSessionImpl.ResourceEntry res : resources) { try { - if (useUri) { - newUrls.add(new URL(res.getUri())); - } else { - newUrls.add(new URL(res.getLocation())); - } + urls[i++] = new URL(res.getUri()); } catch (MalformedURLException e) { incrCounter(LOAD_RESOURCES_ERRORS); log.error("Invalid URL {} with location: {} adding to db {}", res.getUri(), res.getLocation(), database, e); } } - - URLClassLoader newClassLoader = new URLClassLoader(newUrls.toArray(new URL[newUrls.size()]), - DatabaseResourceService.class.getClassLoader()); - if (useUri) { - classLoaderCache.put(database, newClassLoader); - } - - return newClassLoader; - } else { - log.warn("Only URL class loader supported"); - return Thread.currentThread().getContextClassLoader(); } - } - - /** - * Add a resource to the specified database, return class loader with resources added. - * This call does not update the class loader cache - * @param database database name - * @param resources resources which need to be added to the database - * @return class loader updated as a result of adding any JARs - */ - protected ClassLoader loadDBJars(String database, Collection<LensSessionImpl.ResourceEntry> resources) { - return loadDBJars(database, resources, false); + classLoaderCache.put(database, + new UncloseableClassLoader(urls, getClass().getClassLoader())); } @@ -288,9 +221,8 @@ public class DatabaseResourceService extends AbstractService { * Get class loader of a database added with database specific jars * @param database database * @return class loader from cache of classloaders for each db - * @throws LensException */ - protected ClassLoader getClassLoader(String database) throws LensException { + protected ClassLoader getClassLoader(String database) { return classLoaderCache.get(database); } http://git-wip-us.apache.org/repos/asf/lens/blob/df75a4b8/lens-server/src/main/java/org/apache/lens/server/session/LensSessionImpl.java ---------------------------------------------------------------------- diff --git a/lens-server/src/main/java/org/apache/lens/server/session/LensSessionImpl.java b/lens-server/src/main/java/org/apache/lens/server/session/LensSessionImpl.java index 2b84d3a..04812a6 100644 --- a/lens-server/src/main/java/org/apache/lens/server/session/LensSessionImpl.java +++ b/lens-server/src/main/java/org/apache/lens/server/session/LensSessionImpl.java @@ -22,6 +22,8 @@ import java.io.Externalizable; import java.io.IOException; import java.io.ObjectInput; import java.io.ObjectOutput; +import java.net.MalformedURLException; +import java.net.URL; import java.util.*; import java.util.concurrent.atomic.AtomicInteger; @@ -88,19 +90,19 @@ public class LensSessionImpl extends HiveSessionImpl { * List of queries which are submitted in this session. */ @Getter - private List<QueryHandle> activeQueries = new ArrayList<QueryHandle>(); + private final List<QueryHandle> activeQueries = new ArrayList<>(); /** * Keep track of DB static resources which failed to be added to this session */ - private final Map<String, List<ResourceEntry>> failedDBResources = new HashMap<String, List<ResourceEntry>>(); + private final Map<String, List<ResourceEntry>> failedDBResources = new HashMap<>(); /** * Cache of database specific class loaders for this session * This is updated lazily on add/remove resource calls and switch database calls. */ - private final Map<String, ClassLoader> sessionDbClassLoaders = new HashMap<String, ClassLoader>(); + private final Map<String, SessionClassLoader> sessionDbClassLoaders = new HashMap<>(); @Setter(AccessLevel.PROTECTED) private DatabaseResourceService dbResService; @@ -137,9 +139,7 @@ public class LensSessionImpl extends HiveSessionImpl { conf.addResource("lenssession-default.xml"); conf.addResource("lens-site.xml"); sessionDefaultConfig = new Configuration(false); - Iterator<Map.Entry<String, String>> confItr = conf.iterator(); - while (confItr.hasNext()) { - Map.Entry<String, String> prop = confItr.next(); + for (Map.Entry<String, String> prop : conf) { if (!prop.getKey().startsWith(LensConfConstants.SERVER_PFX)) { sessionDefaultConfig.set(prop.getKey(), prop.getValue()); } @@ -202,17 +202,20 @@ public class LensSessionImpl extends HiveSessionImpl { @Override public void close() throws HiveSQLException { + ClassLoader nonDBClassLoader = getSessionState().getConf().getClassLoader(); super.close(); - // Release class loader resources + JavaUtils.closeClassLoadersTo(nonDBClassLoader, getClass().getClassLoader()); synchronized (sessionDbClassLoaders) { - for (Map.Entry<String, ClassLoader> entry : sessionDbClassLoaders.entrySet()) { + for (Map.Entry<String, SessionClassLoader> entry : sessionDbClassLoaders.entrySet()) { try { - // Close the class loader only if its not a class loader maintained by the DB service - if (entry.getValue() != getDbResService().getClassLoader(entry.getKey())) { - // This is a utility in hive-common - JavaUtils.closeClassLoader(entry.getValue()); - } + // Closing session level classloaders up untill the db class loader if present, or null. + // When db class loader is null, the class loader in the session is a single class loader + // which stays as it is on database switch -- provided the new db doesn't have db jars. + // The following line will close class loaders made on top of db class loaders and will close + // only one classloader without closing the parents. In case of no db class loader, the session + // classloader will already have been closed by either super.close() or before this for loop. + JavaUtils.closeClassLoadersTo(entry.getValue(), getDbResService().getClassLoader(entry.getKey())); } catch (Exception e) { log.error("Error closing session classloader for session: {}", getSessionHandle().getSessionId(), e); } @@ -242,7 +245,11 @@ public class LensSessionImpl extends HiveSessionImpl { * @see org.apache.hive.service.cli.session.HiveSessionImpl#acquire() */ public synchronized void acquire() { - super.acquire(true); + this.acquire(true); + } + @Override + public synchronized void acquire(boolean userAccess) { + super.acquire(userAccess); acquireCount.incrementAndGet(); // Update thread's class loader with current DBs class loader ClassLoader classLoader = getClassLoader(getCurrentDatabase()); @@ -256,9 +263,13 @@ public class LensSessionImpl extends HiveSessionImpl { * @see org.apache.hive.service.cli.session.HiveSessionImpl#release() */ public synchronized void release() { + this.release(true); + } + @Override + public synchronized void release(boolean userAccess) { lastAccessTime = System.currentTimeMillis(); if (acquireCount.decrementAndGet() == 0) { - super.release(true); + super.release(userAccess); } } @@ -290,7 +301,8 @@ public class LensSessionImpl extends HiveSessionImpl { itr.remove(); } } - updateSessionDbClassLoader(getSessionState().getCurrentDatabase()); + // New classloaders will be created. Remove resource is expensive, add resource is cheap. + updateAllSessionClassLoaders(); } /** @@ -303,10 +315,9 @@ public class LensSessionImpl extends HiveSessionImpl { public void addResource(String type, String path, String finalLocation) { ResourceEntry resource = new ResourceEntry(type, path, finalLocation); persistInfo.getResources().add(resource); - synchronized (sessionDbClassLoaders) { - // Update all DB class loaders - updateSessionDbClassLoader(getSessionState().getCurrentDatabase()); - } + // The following call updates the existing classloaders without creating new instances. + // Add resource is cheap :) + addResourceToAllSessionClassLoaders(resource); } protected List<ResourceEntry> getResources() { @@ -320,19 +331,71 @@ public class LensSessionImpl extends HiveSessionImpl { public void setCurrentDatabase(String currentDatabase) { persistInfo.setDatabase(currentDatabase); getSessionState().setCurrentDatabase(currentDatabase); - // Merge if resources are added + // Make sure entry is there in classloader cache synchronized (sessionDbClassLoaders) { updateSessionDbClassLoader(currentDatabase); } } + private SessionClassLoader getUpdatedSessionClassLoader(String database) { + ClassLoader dbClassLoader = getDbResService().getClassLoader(database); + if (dbClassLoader == null) { + return null; + } + URL[] urls = new URL[0]; + if (persistInfo.getResources() != null) { + int i = 0; + urls = new URL[persistInfo.getResources().size()]; + for (LensSessionImpl.ResourceEntry res : persistInfo.getResources()) { + try { + urls[i++] = new URL(res.getUri()); + } catch (MalformedURLException e) { + log.error("Invalid URL {} with location: {} adding to db {}", res.getUri(), res.getLocation(), database, e); + } + } + } + if (sessionDbClassLoaders.containsKey(database) + && Arrays.equals(sessionDbClassLoaders.get(database).getURLs(), urls)) { + return sessionDbClassLoaders.get(database); + } + return new SessionClassLoader(urls, dbClassLoader); + } + private void updateSessionDbClassLoader(String database) { - ClassLoader updatedClassLoader = getDbResService().loadDBJars(database, persistInfo.getResources()); + SessionClassLoader updatedClassLoader = getUpdatedSessionClassLoader(database); if (updatedClassLoader != null) { sessionDbClassLoaders.put(database, updatedClassLoader); } } + private void updateAllSessionClassLoaders() { + synchronized (sessionDbClassLoaders) { + // Update all DB class loaders + for (String database: sessionDbClassLoaders.keySet()) { + updateSessionDbClassLoader(database); + } + } + } + + private void addResourceToClassLoader(String database, ResourceEntry res) { + if (sessionDbClassLoaders.containsKey(database)) { + SessionClassLoader sessionClassLoader = sessionDbClassLoaders.get(database); + try { + sessionClassLoader.addURL(new URL(res.getLocation())); + } catch (MalformedURLException e) { + log.error("Invalid URL {} with location: {} adding to db {}", res.getUri(), res.getLocation(), database, e); + } + } + } + private void addResourceToAllSessionClassLoaders(ResourceEntry res) { + synchronized (sessionDbClassLoaders) { + // Update all DB class loaders + for (String database: sessionDbClassLoaders.keySet()) { + addResourceToClassLoader(database, res); + } + } + } + private boolean areResourcesAdded() { return persistInfo.getResources() != null && !persistInfo.getResources().isEmpty(); } @@ -351,25 +414,18 @@ public class LensSessionImpl extends HiveSessionImpl { if (sessionDbClassLoaders.containsKey(database)) { return sessionDbClassLoaders.get(database); } else { - try { - ClassLoader classLoader = getDbResService().getClassLoader(database); - if (classLoader == null) { - log.debug("DB resource service gave null class loader for {}", database); - } else { - if (areResourcesAdded()) { - log.debug("adding resources for {}", database); - // We need to update DB specific classloader with added resources - updateSessionDbClassLoader(database); - classLoader = sessionDbClassLoaders.get(database); - } + ClassLoader classLoader = getDbResService().getClassLoader(database); + if (classLoader == null) { + log.debug("DB resource service gave null class loader for {}", database); + } else { + if (areResourcesAdded()) { + log.debug("adding resources for {}", database); + // We need to update DB specific classloader with added resources + updateSessionDbClassLoader(database); + classLoader = sessionDbClassLoaders.get(database); } - return classLoader == null ? getSessionState().getConf().getClassLoader() : classLoader; - - } catch (LensException e) { - log.error("Error getting classloader for database {} for session {} " - + " defaulting to session state class loader", database, getSessionHandle().getSessionId(), e); - return getSessionState().getConf().getClassLoader(); } + return classLoader == null ? getSessionState().getConf().getClassLoader() : classLoader; } } } @@ -410,13 +466,13 @@ public class LensSessionImpl extends HiveSessionImpl { /** * Return resources which are added statically to the database - * @return + * @return DatabaseResources */ public Collection<ResourceEntry> getDBResources(String database) { synchronized (failedDBResources) { List<ResourceEntry> failed = failedDBResources.get(database); if (failed == null && getDbResService().getResourcesForDatabase(database) != null) { - failed = new ArrayList<ResourceEntry>(getDbResService().getResourcesForDatabase(database)); + failed = new ArrayList<>(getDbResService().getResourcesForDatabase(database)); failedDBResources.put(database, failed); } return failed; @@ -428,7 +484,7 @@ public class LensSessionImpl extends HiveSessionImpl { * Get session's resources which have to be added for the given database */ public Collection<ResourceEntry> getPendingSessionResourcesForDatabase(String database) { - List<ResourceEntry> pendingResources = new ArrayList<ResourceEntry>(); + List<ResourceEntry> pendingResources = new ArrayList<>(); for (ResourceEntry res : persistInfo.getResources()) { if (!res.isAddedToDatabase(database)) { pendingResources.add(res); @@ -439,7 +495,7 @@ public class LensSessionImpl extends HiveSessionImpl { /** * Get effective class loader for this session - * @return + * @return current classloader */ public ClassLoader getClassLoader() { return getClassLoader(getCurrentDatabase()); @@ -503,7 +559,7 @@ public class LensSessionImpl extends HiveSessionImpl { /** * Returns the value of restoreCount for the resource - * @return + * @return restore count */ public int getRestoreCount() { return restoreCount.get(); @@ -541,10 +597,10 @@ public class LensSessionImpl extends HiveSessionImpl { public static class LensSessionPersistInfo implements Externalizable { /** The resources. */ - private List<ResourceEntry> resources = new ArrayList<ResourceEntry>(); + private List<ResourceEntry> resources = new ArrayList<>(); /** The config. */ - private Map<String, String> config = new HashMap<String, String>(); + private Map<String, String> config = new HashMap<>(); /** The session handle. */ private LensSessionHandle sessionHandle; http://git-wip-us.apache.org/repos/asf/lens/blob/df75a4b8/lens-server/src/main/java/org/apache/lens/server/session/SessionClassLoader.java ---------------------------------------------------------------------- diff --git a/lens-server/src/main/java/org/apache/lens/server/session/SessionClassLoader.java b/lens-server/src/main/java/org/apache/lens/server/session/SessionClassLoader.java new file mode 100644 index 0000000..f5e2068 --- /dev/null +++ b/lens-server/src/main/java/org/apache/lens/server/session/SessionClassLoader.java @@ -0,0 +1,55 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.lens.server.session; + +import java.io.IOException; +import java.net.URL; +import java.net.URLClassLoader; + +import lombok.EqualsAndHashCode; +import lombok.Getter; +import lombok.Setter; +import lombok.extern.slf4j.Slf4j; + +@Slf4j +@EqualsAndHashCode(callSuper = false) +public class SessionClassLoader extends URLClassLoader { + @Getter + @Setter + private boolean closed; + + public SessionClassLoader(URL[] urls) { + super(urls); + } + + public SessionClassLoader(URL[] urls, ClassLoader parent) { + super(urls, parent); + } + + @Override + public void close() throws IOException { + setClosed(true); + super.close(); + } + + @Override + public void addURL(URL url) { + super.addURL(url); + } +} http://git-wip-us.apache.org/repos/asf/lens/blob/df75a4b8/lens-server/src/main/java/org/apache/lens/server/session/UncloseableClassLoader.java ---------------------------------------------------------------------- diff --git a/lens-server/src/main/java/org/apache/lens/server/session/UncloseableClassLoader.java b/lens-server/src/main/java/org/apache/lens/server/session/UncloseableClassLoader.java new file mode 100644 index 0000000..c350bcb --- /dev/null +++ b/lens-server/src/main/java/org/apache/lens/server/session/UncloseableClassLoader.java @@ -0,0 +1,41 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.lens.server.session; + +import java.io.IOException; +import java.net.URL; + +import lombok.extern.slf4j.Slf4j; + +@Slf4j +public class UncloseableClassLoader extends SessionClassLoader { + + public UncloseableClassLoader(URL[] urls) { + super(urls); + } + + public UncloseableClassLoader(URL[] urls, ClassLoader parent) { + super(urls, parent); + } + + @Override + public void close() throws IOException { + log.debug("Not closing db class loader"); + } +} http://git-wip-us.apache.org/repos/asf/lens/blob/df75a4b8/lens-server/src/test/java/org/apache/lens/server/session/TestSessionClassLoaders.java ---------------------------------------------------------------------- diff --git a/lens-server/src/test/java/org/apache/lens/server/session/TestSessionClassLoaders.java b/lens-server/src/test/java/org/apache/lens/server/session/TestSessionClassLoaders.java index eeb97f2..d66de4c 100644 --- a/lens-server/src/test/java/org/apache/lens/server/session/TestSessionClassLoaders.java +++ b/lens-server/src/test/java/org/apache/lens/server/session/TestSessionClassLoaders.java @@ -19,7 +19,10 @@ package org.apache.lens.server.session; +import static org.testng.Assert.*; + import java.io.File; +import java.net.URI; import java.net.URLClassLoader; import java.util.HashMap; @@ -27,13 +30,15 @@ import org.apache.lens.api.LensSessionHandle; import org.apache.lens.server.LensServerConf; import org.apache.lens.server.LensServerTestUtil; import org.apache.lens.server.api.LensConfConstants; +import org.apache.lens.server.api.error.LensException; import org.apache.lens.server.user.UserConfigLoaderFactory; import org.apache.hadoop.hive.conf.HiveConf; +import org.apache.hadoop.hive.ql.exec.UDFClassLoader; import org.apache.hadoop.hive.ql.metadata.Hive; import org.apache.hive.service.cli.CLIService; +import org.apache.hive.service.cli.HiveSQLException; -import org.testng.Assert; import org.testng.annotations.AfterClass; import org.testng.annotations.BeforeClass; import org.testng.annotations.Test; @@ -79,6 +84,7 @@ public class TestSessionClassLoaders { public void tearDown() throws Exception { Hive hive = Hive.get(conf); hive.dropDatabase(DB1, true, true); + sessionService.stop(); } @@ -99,7 +105,7 @@ public class TestSessionClassLoaders { Class clz = Class.forName(TestDatabaseResourceService.TEST_CLASS, true, Thread.currentThread().getContextClassLoader()); // Expected to fail - Assert.fail("Should not reach here as default db doesn't have jar loaded"); + fail("Should not reach here as default db doesn't have jar loaded"); } catch (ClassNotFoundException cnf) { // Pass } finally { @@ -112,7 +118,7 @@ public class TestSessionClassLoaders { sessionService.acquire(sessionHandle); Class clz = session.getCubeMetastoreClient().getConf().getClassByName(TestDatabaseResourceService.TEST_CLASS); // Expected to fail - Assert.fail("Should not reach here as default db doesn't have jar loaded"); + fail("Should not reach here as default db doesn't have jar loaded"); } catch (ClassNotFoundException cnf) { // Pass } finally { @@ -127,17 +133,17 @@ public class TestSessionClassLoaders { sessionService.acquire(sessionHandle); ClassLoader thClassLoader = Thread.currentThread().getContextClassLoader(); - Assert.assertTrue(thClassLoader == session.getClassLoader(DB1)); + assertTrue(thClassLoader == session.getClassLoader(DB1)); Class clz = Class.forName(TestDatabaseResourceService.TEST_CLASS, true, thClassLoader); - Assert.assertNotNull(clz); + assertNotNull(clz); // test cube metastore client's configuration's classloader clz = null; clz = session.getCubeMetastoreClient().getConf().getClassByName(TestDatabaseResourceService.TEST_CLASS); - Assert.assertNotNull(clz); + assertNotNull(clz); } catch (ClassNotFoundException cnf) { log.error(cnf.getMessage(), cnf); - Assert.fail("Should not have thrown class not found exception: " + cnf.getMessage()); + fail("Should not have thrown class not found exception: " + cnf.getMessage()); } finally { sessionService.release(sessionHandle); } @@ -169,7 +175,7 @@ public class TestSessionClassLoaders { sessionService.acquire(sessionHandle); ClassLoader dbClassLoader = session.getClassLoader("default"); - Assert.assertTrue(Thread.currentThread().getContextClassLoader() == dbClassLoader); + assertTrue(Thread.currentThread().getContextClassLoader() == dbClassLoader); // testClass2 should be loaded since test2.jar is added to the session Class testClass2 = dbClassLoader.loadClass("ClassLoaderTestClass2"); @@ -181,8 +187,8 @@ public class TestSessionClassLoaders { loadedDBClass = true; } catch (ClassNotFoundException cnf) { log.error(cnf.getMessage(), cnf); - Assert.assertTrue(loadedSessionClass); - Assert.assertFalse(loadedDBClass); + assertTrue(loadedSessionClass); + assertFalse(loadedDBClass); } finally { sessionService.release(sessionHandle); } @@ -204,8 +210,8 @@ public class TestSessionClassLoaders { loadedDBClass = true; } catch (ClassNotFoundException cnf) { log.error(cnf.getMessage(), cnf); - Assert.assertTrue(loadedSessionClass); - Assert.assertFalse(loadedDBClass); + assertTrue(loadedSessionClass); + assertFalse(loadedDBClass); } finally { sessionService.release(sessionHandle); } @@ -217,7 +223,7 @@ public class TestSessionClassLoaders { try { sessionService.acquire(sessionHandle); // testClass2 should be loaded since test2.jar is added to the session - URLClassLoader urlClassLoader = (URLClassLoader) Thread.currentThread().getContextClassLoader(); + URLClassLoader urlClassLoader = (SessionClassLoader) Thread.currentThread().getContextClassLoader(); Class testClass2 = Class.forName("ClassLoaderTestClass2", true, Thread.currentThread().getContextClassLoader()); // class inside 'test.jar' should also load since its added to DB1 loadedSessionClass = true; @@ -226,8 +232,8 @@ public class TestSessionClassLoaders { } finally { sessionService.release(sessionHandle); } - Assert.assertTrue(loadedSessionClass); - Assert.assertTrue(loadedDBClass); + assertTrue(loadedSessionClass); + assertTrue(loadedDBClass); log.info("@@@ TEST 2 - cube client"); loadedSessionClass = false; @@ -243,8 +249,8 @@ public class TestSessionClassLoaders { } finally { sessionService.release(sessionHandle); } - Assert.assertTrue(loadedSessionClass); - Assert.assertTrue(loadedDBClass); + assertTrue(loadedSessionClass); + assertTrue(loadedDBClass); // Switch back to default DB, again the test2.jar should be available, test.jar should not be available log.info("@@@ TEST 3"); @@ -260,8 +266,8 @@ public class TestSessionClassLoaders { Class clz = Class.forName("ClassLoaderTestClass", true, Thread.currentThread().getContextClassLoader()); loadedDBClass = true; } catch (ClassNotFoundException cnf) { - Assert.assertTrue(loadedSessionClass); - Assert.assertFalse(loadedDBClass); + assertTrue(loadedSessionClass); + assertFalse(loadedDBClass); } finally { sessionService.release(sessionHandle); } @@ -282,8 +288,8 @@ public class TestSessionClassLoaders { loadedDBClass = true; } catch (ClassNotFoundException cnf) { log.error(cnf.getMessage(), cnf); - Assert.assertTrue(loadedSessionClass); - Assert.assertFalse(loadedDBClass); + assertTrue(loadedSessionClass); + assertFalse(loadedDBClass); } finally { sessionService.release(sessionHandle); } @@ -291,4 +297,136 @@ public class TestSessionClassLoaders { sessionService.closeSession(sessionHandle); } + private void assertDBClassNotLoading(ClassLoader classLoader) { + try { + classLoader.loadClass("DatabaseJarSerde"); + fail("Shouldn't be able to load DatabaseJarSerde.class"); + } catch (ClassNotFoundException e) { + log.debug("no issues"); + } + } + + private void assertDBClassLoading(ClassLoader classLoader) throws ClassNotFoundException { + classLoader.loadClass("DatabaseJarSerde"); + } + + @Test + public void testClassloaderClose() throws LensException, HiveSQLException, ClassNotFoundException { + String parentResource = "org/apache/hadoop/conf/Configuration.class"; + String db1Resource = "DatabaseJarSerde.class"; + LensSessionHandle sessionHandle1 = sessionService.openSession("foo", "bar", new HashMap<String, String>()); + LensSessionImpl session1 = sessionService.getSession(sessionHandle1); + session1.setDbResService(sessionService.getDatabaseResourceService()); + session1.setCurrentDatabase(DB1); + LensSessionHandle sessionHandle2 = sessionService.openSession("foo", "bar", new HashMap<String, String>()); + LensSessionImpl session2 = sessionService.getSession(sessionHandle2); + session2.setDbResService(sessionService.getDatabaseResourceService()); + session2.setCurrentDatabase(DB1); + SessionClassLoader session1ClassLoader1 = (SessionClassLoader) session1.getClassLoader(); + SessionClassLoader classLoader2 = (SessionClassLoader) session2.getClassLoader(); + // classloader logically same, but instance different + assertEquals(session1ClassLoader1, classLoader2); + assertFalse(session1ClassLoader1 == classLoader2); + assertEquals(session1ClassLoader1.getParent(), classLoader2.getParent()); + assertDBClassLoading(session1ClassLoader1); + session1.setCurrentDatabase("default"); + // Default has no parent class loader, so + UDFClassLoader session1ClassLoader2 = (UDFClassLoader) session1.getClassLoader(); + assertNotEquals(session1ClassLoader2, session1ClassLoader1); // obviously, since even types are different + assertNotEquals(session1ClassLoader1.getParent(), session1ClassLoader2.getParent()); + assertFalse(session1ClassLoader1.isClosed()); + assertFalse(session1ClassLoader2.isClosed()); + assertDBClassNotLoading(session1ClassLoader2); + sessionService.closeSession(sessionHandle1); + // both classloaders got closed, but parent classloader not closed. + assertTrue(session1ClassLoader1.isClosed()); + assertTrue(session1ClassLoader2.isClosed()); + assertFalse(((UncloseableClassLoader) session1ClassLoader1.getParent()).isClosed()); + assertNotNull(session1ClassLoader2.getResource(parentResource)); + // session 1 classloader still able to load db1 jar resources + assertNotNull(session1ClassLoader1.getResource(db1Resource)); + // didn't affect classloaders of another session. + assertNotNull(classLoader2.getResource(parentResource)); + sessionService.closeSession(sessionHandle2); + assertTrue(classLoader2.isClosed()); + LensSessionHandle sessionHandle3 = sessionService.openSession("foo", "bar", new HashMap<String, String>()); + LensSessionImpl session3 = sessionService.getSession(sessionHandle3); + session3.setDbResService(sessionService.getDatabaseResourceService()); + session3.setCurrentDatabase("default"); + UDFClassLoader session3ClassLoader1 = (UDFClassLoader) session3.getClassLoader(); + assertFalse(session3ClassLoader1.isClosed()); + assertDBClassNotLoading(session3ClassLoader1); + session3.setCurrentDatabase(DB1); + SessionClassLoader session3ClassLoader2 = (SessionClassLoader) session3.getClassLoader(); + assertFalse(session3ClassLoader2.isClosed()); + assertDBClassLoading(session3ClassLoader2); + // session classloader is different in case of different sessions. + assertNotEquals(session3ClassLoader1, session1ClassLoader2); + // both are instances of UDFClassLoader, with same number of urls + assertEquals(session3ClassLoader1.getClass(), session1ClassLoader2.getClass()); + assertEquals(session1ClassLoader2.getURLs().length, session3ClassLoader1.getURLs().length); + // without adding any jars, classloaders for different sessions using same database have same + // parent and no extra urls added + assertEquals(session3ClassLoader2.getParent(), session1ClassLoader1.getParent()); + assertEquals(session3ClassLoader2.getURLs().length, session1ClassLoader1.getURLs().length); + + session3.setCurrentDatabase("dummy1"); + UDFClassLoader session3ClassLoader3 = (UDFClassLoader) session3.getClassLoader(); + session3.setCurrentDatabase("dummy2"); + UDFClassLoader session3ClassLoader4 = (UDFClassLoader) session3.getClassLoader(); + assertDBClassNotLoading(session3ClassLoader3); + assertDBClassNotLoading(session3ClassLoader4); + assertEquals(session3ClassLoader3, session3ClassLoader4); + sessionService.closeSession(sessionHandle3); + assertTrue(session3ClassLoader1.isClosed()); + assertTrue(session3ClassLoader2.isClosed()); + assertTrue(session3ClassLoader3.isClosed()); + assertTrue(session3ClassLoader4.isClosed()); + } + + private void assertExtraClassNotLoading(ClassLoader classLoader) { + try { + classLoader.loadClass("ClassLoaderTestClass2"); + // Class may be already loaded due to cache. + assertNull(classLoader.getResource("ClassLoaderTestClass2.class")); + } catch (ClassNotFoundException e) { + log.debug("no issues"); + } + } + + private void assertExtraClassLoading(ClassLoader classLoader) throws ClassNotFoundException { + classLoader.loadClass("ClassLoaderTestClass2"); + assertNotNull("ClassLoaderTestClass2.class"); + } + + @Test + public void testSessionClassLoaderCloseWhenExtraJars() throws Exception { + URI resource = new File("target/testjars/test2.jar").toURI(); + LensSessionHandle sessionHandle1 = sessionService.openSession("foo", "bar", new HashMap<String, String>()); + LensSessionImpl session1 = sessionService.getSession(sessionHandle1); + session1.setDbResService(sessionService.getDatabaseResourceService()); + session1.setCurrentDatabase(DB1); + sessionService.addResource(sessionHandle1, "jar", resource.toString()); + SessionClassLoader loader1 = (SessionClassLoader) session1.getClassLoader(); + assertExtraClassLoading(loader1); + assertExtraClassNotLoading(loader1.getParent()); + assertEquals(loader1.getURLs()[0].toURI(), resource); + // change db to a db which doesn't have db jars. and add resources there. + + session1.setCurrentDatabase("default"); + // Extra jar is still there. + assertExtraClassLoading(session1.getClassLoader()); + UDFClassLoader loader2 = (UDFClassLoader) session1.getClassLoader(); + + // Close and assert on all class loaders. + session1.setCurrentDatabase(DB1); + assertEquals(session1.getClassLoader(), loader1); + sessionService.closeSession(sessionHandle1); + assertTrue(loader1.isClosed()); + assertTrue(loader2.isClosed()); + assertExtraClassNotLoading(loader1); + assertExtraClassNotLoading(loader2); + assertFalse(((UncloseableClassLoader) loader1.getParent()).isClosed()); + assertExtraClassNotLoading(loader1.getParent()); + } }
