keith-turner commented on code in PR #33:
URL:
https://github.com/apache/accumulo-classloaders/pull/33#discussion_r2590453604
##########
pom.xml:
##########
@@ -411,7 +411,7 @@ under the License.
<reactorModuleConvergence />
<banDuplicatePomDependencyVersions />
<dependencyConvergence />
- <banDynamicVersions />
+ <!-- <banDynamicVersions />-->
Review Comment:
Could create a reminder issue for this.
##########
modules/local-caching-classloader/src/main/java/org/apache/accumulo/classloader/lcc/LocalCachingContextCleaner.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
+ *
+ * https://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.accumulo.classloader.lcc;
+
+import java.io.IOException;
+import java.lang.ref.Cleaner;
+import java.lang.ref.SoftReference;
+import java.net.URLClassLoader;
+import java.util.ArrayList;
+import java.util.List;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class LocalCachingContextCleaner {
+
+ private static final Logger LOG =
LoggerFactory.getLogger(LocalCachingContextCleaner.class);
+ private static final List<SoftReference<URLClassLoader>> LOADERS = new
ArrayList<>();
Review Comment:
A WeakRef for this list seems good. As long as the classloader is refereced
by the caffeine cache the weak ref will be alive. The javadocs for weak ref
mention the following also.
```
Weak reference objects, which do not prevent their referents from being made
finalizable, finalized, and then reclaimed.
```
##########
modules/local-caching-classloader/README.md:
##########
@@ -90,7 +93,12 @@ create the classloader and return the exception to the
calling code.
Because the cache directory is shared among multiple processes, and one
process can't know what the other processes are doing,
this class cannot clean up the shared cache directory. It is left to the user
to remove unused context cache directories
-and unused old files within a context cache directory.
+and unused old files within a context cache directory. To aid in this task a
JMX MXBean has been created to expose the
+files that are still referenced by the classloaders that are created. For an
example of how to use this MXBean, please
+see the test method
`MiniAccumuloClusterClassLoaderFactoryTest.getReferencedFiles`. This method
attaches to the
Review Comment:
This could be a link to the test class.
```suggestion
see the test method
[MiniAccumuloClusterClassLoaderFactoryTest.getReferencedFiles](src/main/test/.../MiniAccumuloClusterClassLoaderFactoryTest.java).
This method attaches to the
```
##########
modules/local-caching-classloader/README.md:
##########
@@ -90,7 +93,12 @@ create the classloader and return the exception to the
calling code.
Because the cache directory is shared among multiple processes, and one
process can't know what the other processes are doing,
this class cannot clean up the shared cache directory. It is left to the user
to remove unused context cache directories
-and unused old files within a context cache directory.
+and unused old files within a context cache directory. To aid in this task a
JMX MXBean has been created to expose the
+files that are still referenced by the classloaders that are created. For an
example of how to use this MXBean, please
+see the test method
`MiniAccumuloClusterClassLoaderFactoryTest.getReferencedFiles`. This method
attaches to the
+local Accumulo JVM processes to get the set of referenced files. It should be
safe to delete files that are located
+in the base cache directory (set by property
`general.custom.classloader.lcc.cache.dir`) that are NOT in the set
+of referenced files.
Review Comment:
```suggestion
of referenced files and existed before references were gathered.
```
--
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]
