Add tools for ContextDefinition create and cleanup #33

dlmarion · 2025-12-03T18:40:45Z

Modified ContextDefinition to implement KeywordExecutable so that the user can use accumulo create-context-definition to create the json required for the ContextDefinition file.

Added a Cleaner to call URLClassLoader.close on instances that are about to be garbage collected. The LocalCachingContextCleaner is used to register the URLClassLoader with the Cleaner and this class also keeps references to the URLClassLoaders. The list is accessed from a MXBean so that a user can use JMX to retrieve a list of files that are referenced in the local cache directory from the classloaders that have been created but not yet gc'd. This should aid users in determining which files can be removed from the local cache directory.

Modified ContextDefinition to implement KeywordExecutable so that the user can use `accumulo create-context-definition` to create the json required for the ContextDefinition file. Added a Cleaner to call `URLClassLoader.close` on instances that are about to be garbage collected. The LocalCachingContextCleaner is used to register the URLClassLoader with the Cleaner and this class also keeps references to the URLClassLoaders. The list is accessed from a MXBean so that a user can use JMX to retrieve a list of files that are referenced in the local cache directory from the classloaders that have been created but not yet gc'd. This should aid users in determining which files can be removed from the local cache directory.

dlmarion · 2025-12-03T18:42:38Z

...test/java/org/apache/accumulo/classloader/lcc/LocalCachingContextClassLoaderFactoryTest.java

    final URL invalidDefUrl = new URL(fs.getUri().toString() + invalid.toUri().toString());

-    ContextClassLoaderException ex = assertThrows(ContextClassLoaderException.class,
+    assertThrows(ContextClassLoaderException.class,


The error changed when I set GSON to use pretty printing. Not sure the exact error message is a concern here.

dlmarion · 2025-12-03T18:44:22Z

...test/java/org/apache/accumulo/classloader/lcc/MiniAccumuloClusterClassLoaderFactoryTest.java

    @Override
    public void configureMiniCluster(MiniAccumuloConfigImpl cfg,
        org.apache.hadoop.conf.Configuration coreSite) {
+      cfg.removeJvmOption("-XX:+PerfDisableSharedMem");


This requires apache/accumulo#5999 to be merged. This JVM option was not allowing the test to attach to the locally running Accumulo JVM processes. Additionally, I think this is what causes jps not to report the processes.

dlmarion · 2025-12-03T18:45:09Z

pom.xml

                <banDuplicatePomDependencyVersions />
                <dependencyConvergence />
-                <banDynamicVersions />
+                <!--                <banDynamicVersions />-->


Can revert this when 2.1.5 is released and the version changed above.

What in 2.1.5 does this PR depend on that is not in 2.1.4?

Could create a reminder issue for this.

dlmarion · 2025-12-03T18:50:26Z

This won't build successfully until apache/accumulo#5999 is merged. Tests pass locally for me.

dlmarion · 2025-12-04T18:18:46Z

...lassloader/src/main/java/org/apache/accumulo/classloader/lcc/LocalCachingContextCleaner.java

+  public static void registerClassLoader(final URLClassLoader cl) {
+    LOADERS.add(new SoftReference<>(cl));
+    CLEANER.register(cl, () -> {
+      LOADERS.removeIf((sr) -> sr.get() == cl);


The SoftReference may be cleared when this is called. May just want to remove if SoftReference.get() returns null.

dlmarion · 2025-12-04T18:27:54Z

...lassloader/src/main/java/org/apache/accumulo/classloader/lcc/LocalCachingContextCleaner.java

+public class LocalCachingContextCleaner {
+
+  private static final Logger LOG = LoggerFactory.getLogger(LocalCachingContextCleaner.class);
+  private static final List<SoftReference<URLClassLoader>> LOADERS = new ArrayList<>();


Change to WeakReference?

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.

I agree with replacing with WeakRef. From what I understand, if no strong refs exist, weak refs will be removed on next GC, but soft refs will only be removed on GC when/if memory is scarce. So things like getReferencedClassLoaders() will be more accurate with weak ref

ctubbsii

I didn't quite understand why this needs to depend on 2.1.5-SNAPSHOT that isn't available in 2.1.4.

ctubbsii · 2025-12-04T18:17:06Z

...sloader/src/main/resources/META-INF/services/org.apache.accumulo.start.spi.KeywordExecutable

+# under the License.
+#
+
+org.apache.accumulo.classloader.lcc.definition.ContextDefinition


Manually adding this file is fine, but you could also have used the auto-service plugin with the annotation to generate this during the build, so we have a little more flexibility and can be assured that the file will be updated if/when we change the class name, add more, etc.

ctubbsii · 2025-12-04T18:30:41Z

...lassloader/src/main/java/org/apache/accumulo/classloader/lcc/LocalCachingContextCleaner.java

+    LOADERS.add(new SoftReference<>(cl));
+    CLEANER.register(cl, () -> {


This won't work. SoftReferences prevent the monitor object from becoming phantom-reachable, a prerequisite to run the cleaner. Instead, look at what I did in the CleanerUtil class in the main Accumulo code. In order to clean up the Closeable, it has to be contained within the monitor object that becomes phantom-reachable. And then, you can close the objects that were contained within it. The main catch is that you can't have anything registered with the cleaner referencing the object being monitored. So, you can't close the object being monitored this way, but you can close what's inside it. So, you can close a URLClassLoader held inside a LocalCachingContext, if the LocalCachingContext becomes phantom-reachable, but you can't watch for the URLClassLoader to be phantom reachable and then close it.

ctubbsii · 2025-12-04T18:32:35Z

...src/main/java/org/apache/accumulo/classloader/lcc/LocalCachingContextClassLoaderFactory.java

+    try {
+      MBeanServer mbs = ManagementFactory.getPlatformMBeanServer();
+      mbs.registerMBean(new ContextClassLoaders(), ContextClassLoadersMXBean.getObjectName());
+    } catch (MalformedObjectNameException | MBeanRegistrationException
+        | NotCompliantMBeanException e) {
+      throw new IllegalStateException("Error registering MBean", e);
+    } catch (InstanceAlreadyExistsException e) {
+      // instance was re-init'd. This is likely to happen during tests
+      // can ignore as no issue here
+    }


What's this stuff for?

keith-turner · 2025-12-04T20:06:33Z

modules/local-caching-classloader/README.md

+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.


Suggested change

of referenced files.

of referenced files and existed before references were gathered.

keith-turner · 2025-12-04T20:15:30Z

modules/local-caching-classloader/README.md

-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


This could be a link to the test class.

Suggested change

see the test method `MiniAccumuloClusterClassLoaderFactoryTest.getReferencedFiles`. This method attaches to the

see the test method [MiniAccumuloClusterClassLoaderFactoryTest.getReferencedFiles](src/main/test/.../MiniAccumuloClusterClassLoaderFactoryTest.java). This method attaches to the

keith-turner · 2025-12-04T20:21:14Z

pom.xml

                <banDuplicatePomDependencyVersions />
                <dependencyConvergence />
-                <banDynamicVersions />
+                <!--                <banDynamicVersions />-->


Could create a reminder issue for this.

keith-turner · 2025-12-04T20:29:10Z

...lassloader/src/main/java/org/apache/accumulo/classloader/lcc/LocalCachingContextCleaner.java

+public class LocalCachingContextCleaner {
+
+  private static final Logger LOG = LoggerFactory.getLogger(LocalCachingContextCleaner.class);
+  private static final List<SoftReference<URLClassLoader>> LOADERS = new ArrayList<>();


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.

keith-turner · 2025-12-04T20:31:54Z

...lassloader/src/main/java/org/apache/accumulo/classloader/lcc/LocalCachingContextCleaner.java

+
+  public static List<URLClassLoader> getReferencedClassLoaders() {
+    List<URLClassLoader> cll = new ArrayList<>();
+    LOADERS.forEach(sr -> cll.add(sr.get()));


This should not add things where ref.get() returns null.

keith-turner · 2025-12-04T20:35:21Z

...lassloader/src/main/java/org/apache/accumulo/classloader/lcc/LocalCachingContextCleaner.java

+
+  public static void registerClassLoader(final URLClassLoader cl) {
+    LOADERS.add(new SoftReference<>(cl));
+    CLEANER.register(cl, () -> {


Not sure if cleaner is needed, but also not too familiar w/ cleaners. Could do the following to just remove any garbage whenever something is added. Seems like that would work well w/ weak refs.

Suggested change

CLEANER.register(cl, () -> {

LOADERS.removeIf(ref->ref.get() == null);

keith-turner · 2025-12-04T20:37:29Z

...lassloader/src/main/java/org/apache/accumulo/classloader/lcc/LocalCachingContextCleaner.java

+public class LocalCachingContextCleaner {
+
+  private static final Logger LOG = LoggerFactory.getLogger(LocalCachingContextCleaner.class);
+  private static final List<SoftReference<URLClassLoader>> LOADERS = new ArrayList<>();


Suggested change

private static final List<SoftReference<URLClassLoader>> LOADERS = new ArrayList<>();

private static final List<SoftReference<URLClassLoader>> LOADERS = Collections.synchronizedList(new ArrayList<>());

kevinrr888 · 2025-12-04T23:05:07Z

...test/java/org/apache/accumulo/classloader/lcc/LocalCachingContextClassLoaderFactoryTest.java

+    assertThrows(ContextClassLoaderException.class,
        () -> FACTORY.getClassLoader(invalidDefUrl.toString()));


Current change is fine, but it would probably be good to still ensure the cause is a JSON problem. Could do something like the following

Suggested change

assertThrows(ContextClassLoaderException.class,

() -> FACTORY.getClassLoader(invalidDefUrl.toString()));

Throwable exception = assertThrows(ContextClassLoaderException.class,

() -> FACTORY.getClassLoader(invalidDefUrl.toString()));

// ensure json is somewhere in the exception

outer: while (exception != null) {

for (var trace : exception.getStackTrace()) {

if (trace.toString().contains("com.google.gson")) {

break outer;

}

}

exception = exception.getCause();

}

assertNotNull(exception);

dlmarion requested review from ctubbsii, ddanielr, keith-turner and kevinrr888 December 3, 2025 18:40

dlmarion self-assigned this Dec 3, 2025

dlmarion commented Dec 3, 2025

View reviewed changes

Add -Dapache.snapshots=true to MAVEN_OPTS in GH workflows

a8a059b

dlmarion mentioned this pull request Dec 4, 2025

VFSClassLoader alternative #30

Merged

Simplified code in ContextDefinition

c674f77

dlmarion commented Dec 4, 2025

View reviewed changes

ctubbsii reviewed Dec 4, 2025

View reviewed changes

keith-turner reviewed Dec 4, 2025

View reviewed changes

kevinrr888 reviewed Dec 4, 2025

View reviewed changes

		LOADERS.add(new SoftReference<>(cl));
		CLEANER.register(cl, () -> {

	of referenced files.
	of referenced files and existed before references were gathered.

	see the test method `MiniAccumuloClusterClassLoaderFactoryTest.getReferencedFiles`. This method attaches to the
	see the test method [MiniAccumuloClusterClassLoaderFactoryTest.getReferencedFiles](src/main/test/.../MiniAccumuloClusterClassLoaderFactoryTest.java). This method attaches to the

	CLEANER.register(cl, () -> {
	LOADERS.removeIf(ref->ref.get() == null);

	private static final List<SoftReference<URLClassLoader>> LOADERS = new ArrayList<>();
	private static final List<SoftReference<URLClassLoader>> LOADERS = Collections.synchronizedList(new ArrayList<>());

		assertThrows(ContextClassLoaderException.class,
		() -> FACTORY.getClassLoader(invalidDefUrl.toString()));

-    assertThrows(ContextClassLoaderException.class,
-        () -> FACTORY.getClassLoader(invalidDefUrl.toString()));
+    Throwable exception = assertThrows(ContextClassLoaderException.class,
+() -> FACTORY.getClassLoader(invalidDefUrl.toString()));
+    // ensure json is somewhere in the exception
+    outer: while (exception != null) {
+      for (var trace : exception.getStackTrace()) {
+        if (trace.toString().contains("com.google.gson")) {
+          break outer;
+        }
+      }
+      exception = exception.getCause();
+    }
+    assertNotNull(exception);

Add tools for ContextDefinition create and cleanup #33

Are you sure you want to change the base?

Add tools for ContextDefinition create and cleanup #33

Uh oh!

Conversation

dlmarion commented Dec 3, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dlmarion commented Dec 3, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ctubbsii left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants