ctubbsii commented on code in PR #35:
URL:
https://github.com/apache/accumulo-classloaders/pull/35#discussion_r2675029713
##########
modules/local-caching-classloader/src/main/java/org/apache/accumulo/classloader/lcc/LocalCachingContextClassLoaderFactory.java:
##########
@@ -266,10 +194,106 @@ public ClassLoader getClassLoader(final String
contextLocation)
} catch (RuntimeException e) {
throw new ContextClassLoaderException(e.getMessage(), e);
}
- if (newlyCreated.get()) {
- monitorContext(contextLocation, def.getMonitorIntervalSeconds());
+ return classloader.get();
+ }
+
+ private ContextDefinition computeDefinitionAndClassLoader(
+ AtomicReference<URLClassLoader> resultHolder, String contextLocation,
+ ContextDefinition previousDefinition) {
+ ContextDefinition computedDefinition;
+ if (previousDefinition == null) {
+ try {
+ computedDefinition = parseContextDefinition(contextLocation);
+ monitorContext(contextLocation,
computedDefinition.getMonitorIntervalSeconds());
+ } catch (ContextClassLoaderException e) {
+ throw new WrappedException(e);
+ }
+ } else {
+ computedDefinition = previousDefinition;
}
- return cl.get();
+ final URLClassLoader classloader = classloaders.computeIfAbsent(
+ computedDefinition.getContextName() + "-" +
computedDefinition.getChecksum(),
+ (Supplier<URLClassLoaderParams>) () -> {
+ try {
+ return localStore.get().storeContextResources(computedDefinition);
+ } catch (Exception e) {
+ throw new WrappedException(e);
+ }
+ });
+ resultHolder.set(classloader);
+ return computedDefinition;
+ }
+
+ private void checkMonitoredLocation(String contextLocation, long interval) {
+ ContextDefinition currentDef =
+ contextDefs.compute(contextLocation, (contextLocationKey,
previousDefinition) -> {
+ if (previousDefinition == null) {
+ return null;
+ }
+ if (!classloaders.anyMatch(k2 -> k2.substring(0, k2.lastIndexOf('-'))
+ .equals(previousDefinition.getContextName()))) {
+ // context has been removed from the map, no need to check for
update
+ LOG.debug("ClassLoader for context {} not present, no longer
monitoring for changes",
+ contextLocation);
+ return null;
+ }
+ return previousDefinition;
+ });
+ if (currentDef == null) {
+ // context has been removed from the map, no need to check for update
+ LOG.debug("ContextDefinition for context {} not present, no longer
monitoring for changes",
+ contextLocation);
+ return;
+ }
+ long nextInterval = interval;
+ try {
+ final ContextDefinition update = parseContextDefinition(contextLocation);
+ if (!currentDef.getChecksum().equals(update.getChecksum())) {
+ LOG.debug("Context definition for {} has changed", contextLocation);
+ if (!currentDef.getContextName().equals(update.getContextName())) {
+ LOG.warn(
+ "Context name changed for context {}, but context cache
directory will remain {} (old={}, new={})",
+ contextLocation, currentDef.getContextName(),
currentDef.getContextName(),
+ update.getContextName());
+ }
+ localStore.get().storeContextResources(update);
+ contextDefs.put(contextLocation, update);
+ nextInterval = update.getMonitorIntervalSeconds();
+ classloaderFailures.remove(contextLocation);
+ } else {
+ LOG.trace("Context definition for {} has not changed",
contextLocation);
+ }
+ } catch (ContextClassLoaderException | InterruptedException | IOException
| URISyntaxException
+ | RuntimeException e) {
+ LOG.error("Error parsing updated context definition at {}. Classloader
NOT updated!",
+ contextLocation, e);
+ final Timer failureTimer = classloaderFailures.get(contextLocation);
+ if (updateFailureGracePeriodMins.isZero()) {
+ // failure monitoring is disabled
+ LOG.debug("Property {} not set, not tracking classloader failures for
context {}",
Review Comment:
I think it was probably debug because this will log for every context. For
this property, it probably makes sense to be at "INFO" level and only log once,
since it's not really a problem to warn about, but the natural behavior of the
user's choice. We can log the user's choice once when we start, if the property
isn't set, but I don't think we need to do that again for every context. If
we're doing something for every context, as this currently does, debug is
probably fine.
--
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]
