ctubbsii commented on code in PR #35:
URL:
https://github.com/apache/accumulo-classloaders/pull/35#discussion_r2677309537
##########
modules/local-caching-classloader/src/main/java/org/apache/accumulo/classloader/lcc/LocalCachingContextClassLoaderFactory.java:
##########
@@ -206,70 +121,168 @@ void resetForTests() {
@Override
public void init(ContextClassLoaderEnvironment env) {
- baseCacheDir =
requireNonNull(env.getConfiguration().get(Constants.CACHE_DIR_PROPERTY),
+ String value =
requireNonNull(env.getConfiguration().get(Constants.CACHE_DIR_PROPERTY),
"Property " + Constants.CACHE_DIR_PROPERTY + " not set, cannot create
cache directory.");
String graceProp =
env.getConfiguration().get(Constants.UPDATE_FAILURE_GRACE_PERIOD_MINS);
long graceMins = graceProp == null ? 0 : Long.parseLong(graceProp);
updateFailureGracePeriodMins = Duration.ofMinutes(graceMins);
+ final Path baseCacheDir;
+ if (value.startsWith("file:")) {
+ try {
+ baseCacheDir = Path.of(new URL(value).toURI());
+ } catch (IOException | URISyntaxException e) {
+ throw new IllegalArgumentException(
+ "Malformed file: URL specified for base directory: " + value, e);
+ }
+ } else if (value.startsWith("/")) {
+ baseCacheDir = Path.of(value);
+ } else {
+ throw new IllegalArgumentException(
+ "Base directory is neither a file URL nor an absolute file path: " +
value);
+ }
try {
- CacheUtils.createBaseCacheDir(baseCacheDir);
- } catch (IOException | ContextClassLoaderException e) {
- throw new IllegalStateException("Error creating base cache directory at
" + baseCacheDir, e);
+ localStore.set(new LocalStore(baseCacheDir));
+ } catch (IOException e) {
+ throw new UncheckedIOException("Unable to create the local storage area
at " + baseCacheDir,
+ e);
}
}
- ConcurrentHashMap<String,ContextDefinition> contextDefs = new
ConcurrentHashMap<>();
-
@Override
public ClassLoader getClassLoader(final String contextLocation)
throws ContextClassLoaderException {
- Preconditions.checkState(baseCacheDir != null, "init not called before
calling getClassLoader");
- requireNonNull(contextLocation, "context name must be supplied");
- final AtomicBoolean newlyCreated = new AtomicBoolean(false);
- final AtomicReference<URLClassLoader> cl = new AtomicReference<>();
- ContextDefinition def;
+ Preconditions.checkState(localStore.get() != null,
+ "init not called before calling getClassLoader");
+ requireNonNull(contextLocation, "context location must be supplied");
+ final var classloader = new AtomicReference<URLClassLoader>();
try {
- def = contextDefs.compute(contextLocation, (k, v) -> {
- ContextDefinition def2;
- if (v == null) {
- newlyCreated.set(true);
+ // get the current definition, or create it from the location if it
doesn't exist; this has
+ // the side effect of creating and caching a URLClassLoader instance if
it doesn't exist for
+ // the computed definition
+ contextDefs.compute(contextLocation,
+ (contextLocationKey, previousDefinition) ->
computeDefinitionAndClassLoader(classloader,
+ contextLocationKey, previousDefinition));
+ } catch (RuntimeException e) {
+ throw new ContextClassLoaderException(e.getMessage(), e);
+ }
+ return classloader.get();
+ }
+
+ private ContextDefinition computeDefinitionAndClassLoader(
+ AtomicReference<URLClassLoader> resultHolder, String contextLocation,
+ ContextDefinition previousDefinition) {
+ ContextDefinition computedDefinition;
+ if (previousDefinition == null) {
+ try {
+ URL url = new URL(contextLocation);
+ computedDefinition = ContextDefinition.fromRemoteURL(url);
+ monitorContext(contextLocation,
computedDefinition.getMonitorIntervalSeconds());
Review Comment:
The monitor task won't actually run before this finishes the initial setup,
because it is blocked by the contextDefs.compute() on that URL. There are some
comments explaining this a bit, but this is the part I hate the most about my
implementation, because it's not very clear. If there's a way to improve this,
either by additional comments or refactoring, I'd be interested.
--
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]
