keith-turner commented on code in PR #33:
URL:
https://github.com/apache/accumulo-classloaders/pull/33#discussion_r2695416209
##########
modules/local-caching-classloader/README.md:
##########
@@ -170,20 +177,28 @@ constructed at that time.
## Cleanup
+
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 of unused resources. It is left to the user to remove
unused files from the cache. While the context definition JSON files are always
safe to delete, it is not recommended to do so for any that are still in use,
because they can be useful for troubleshooting.
+To aid in this task a JMX MXBean has been created to expose the files that are
still referenced
Review Comment:
This offers the building blocks to create a cleanup utility and leave that
as an exercise to the reader. It would probably be best (in a follow on change)
to write a cleanup utility in this project that uses these building blocks, for
the following reasons.
* Can document and test the cleanup code in one place. If cleanup code is
written, seems suboptimal for it to live in another repo or for there to be
multiple implementions.
* Having a working end to end impl gives confidence that these building
blocks provide everything that is needed to do cleanup.
* The cleanup code will need to have some awareness of the directory layout
of the cache dir which is an internal implementation detail.
* The clean up utility will have to get the algorithm correct in order to
avoid race conditions. Seems like its important to list the dir first prior to
get refs from all processes, but not completely sure. Would need to experiment
and see the code to be sure.
* Could make the cleanup code more robust by failing properly if no JMX
seen or mismatch in dir passed in and dirs seen from jmx.
We could make the new command line utility have subcommands to support this.
Something like the following.
```
accumulo context-classloader --help
# create context definition json and print to stdout
accumulo context-classloader create -i <interval> -f {files}
# remove files not in use by any of the given pids
accumulo context-classloader cleanup <cache dir> {pids}
```
--
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]
