cshannon commented on code in PR #3136:
URL: https://github.com/apache/accumulo/pull/3136#discussion_r1089756846
##########
core/src/main/java/org/apache/accumulo/core/classloader/DefaultContextClassLoaderFactory.java:
##########
@@ -48,55 +46,49 @@ public class DefaultContextClassLoaderFactory implements
ContextClassLoaderFacto
private static final Logger LOG =
LoggerFactory.getLogger(DefaultContextClassLoaderFactory.class);
private static final String className =
DefaultContextClassLoaderFactory.class.getName();
- @SuppressWarnings("removal")
- private static final Property VFS_CONTEXT_CLASSPATH_PROPERTY =
- Property.VFS_CONTEXT_CLASSPATH_PROPERTY;
+ // Do we set a max size here and how long until we expire the classloader?
+ private final Cache<String,Context> contexts =
+ Caffeine.newBuilder().maximumSize(100).expireAfterAccess(1,
TimeUnit.DAYS).build();
- public DefaultContextClassLoaderFactory(final AccumuloConfiguration accConf)
{
+ public DefaultContextClassLoaderFactory() {
if (!isInstantiated.compareAndSet(false, true)) {
throw new IllegalStateException("Can only instantiate " + className + "
once");
}
- Supplier<Map<String,String>> contextConfigSupplier =
- () ->
accConf.getAllPropertiesWithPrefix(VFS_CONTEXT_CLASSPATH_PROPERTY);
- setContextConfig(contextConfigSupplier);
- LOG.debug("ContextManager configuration set");
- startCleanupThread(accConf, contextConfigSupplier);
}
- @SuppressWarnings("deprecation")
- private static void setContextConfig(Supplier<Map<String,String>>
contextConfigSupplier) {
- org.apache.accumulo.start.classloader.vfs.AccumuloVFSClassLoader
- .setContextConfig(contextConfigSupplier);
- }
+ @Override
+ public ClassLoader getClassLoader(String contextName) {
+ if (contextName == null) {
+ throw new IllegalArgumentException("Unknown context");
+ }
- private static void startCleanupThread(final AccumuloConfiguration conf,
- final Supplier<Map<String,String>> contextConfigSupplier) {
- ScheduledFuture<?> future = ThreadPools.getClientThreadPools((t, e) -> {
- LOG.error("context classloader cleanup thread has failed.", e);
- }).createGeneralScheduledExecutorService(conf)
- .scheduleWithFixedDelay(Threads.createNamedRunnable(className +
"-cleanup", () -> {
- LOG.trace("{}-cleanup thread, properties: {}", className, conf);
- Set<String> contextsInUse =
contextConfigSupplier.get().keySet().stream()
- .map(p ->
p.substring(VFS_CONTEXT_CLASSPATH_PROPERTY.getKey().length()))
- .collect(Collectors.toSet());
- LOG.trace("{}-cleanup thread, contexts in use: {}", className,
contextsInUse);
- removeUnusedContexts(contextsInUse);
- }), 1, 1, MINUTES);
- ThreadPools.watchNonCriticalScheduledTask(future);
- LOG.debug("Context cleanup timer started at 60s intervals");
- }
+ final ClassLoader loader = contexts.get(contextName,
Context::new).getClassLoader();
- @SuppressWarnings("deprecation")
- private static void removeUnusedContexts(Set<String> contextsInUse) {
- org.apache.accumulo.start.classloader.vfs.AccumuloVFSClassLoader
- .removeUnusedContexts(contextsInUse);
+ LOG.debug("Returning classloader {} for context {}",
loader.getClass().getName(), contextName);
+ return loader;
}
- @SuppressWarnings("deprecation")
- @Override
- public ClassLoader getClassLoader(String contextName) {
- return org.apache.accumulo.start.classloader.vfs.AccumuloVFSClassLoader
- .getContextClassLoader(contextName);
+ private static class Context {
Review Comment:
The purpose of Caffeine was so that the cache will remove old classloaders
automatically when no longer needed but this could be re-worked if desired.
In the current version there is a thread that runs to check for no longer
needed contexts and will remove them. The idea of using Caffeine was that we
can cache the Classloader but if it is no longer needed anymore after some
period of time (nothing reads it because maybe the config was dropped, etc) we
can then just remove it from the cache automatically. This seemed simpler than
maintaining a GC thread.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
