ctubbsii commented on code in PR #58:
URL:
https://github.com/apache/accumulo-classloaders/pull/58#discussion_r2761273384
##########
modules/local-caching-classloader/src/main/java/org/apache/accumulo/classloader/lcc/util/LccUtils.java:
##########
@@ -33,18 +49,89 @@ public class LccUtils {
private static final Logger LOG = LoggerFactory.getLogger(LccUtils.class);
private static final ConcurrentHashMap<String,DigestUtils> DIGESTERS = new
ConcurrentHashMap<>();
+ private static final Cleaner CLEANER = Cleaner.create();
// keep at most one DigestUtils instance for each algorithm
public static DigestUtils getDigester(String algorithm) {
return DIGESTERS.computeIfAbsent(algorithm, DigestUtils::new);
}
+ private static String checksumForFileName(String algorithm, String checksum)
{
+ return algorithm.replace('/', '_') + "-" + checksum;
+ }
+
+ public static String checksumForFileName(ContextDefinition definition) {
+ return checksumForFileName(definition.getChecksumAlgorithm(),
definition.getChecksum());
+ }
+
+ public static String checksumForFileName(Resource definition) {
+ return checksumForFileName(definition.getAlgorithm(),
definition.getChecksum());
+ }
+
@SuppressFBWarnings(value = "DP_CREATE_CLASSLOADER_INSIDE_DO_PRIVILEGED",
justification = "doPrivileged is deprecated without replacement and
removed in newer Java")
- public static URLClassLoader createClassLoader(String name, URL[] urls) {
- final var cl = new URLClassLoader(name, urls,
+ public static URLClassLoader createClassLoader(ContextCacheKey cacheKey,
+ URLClassLoaderParams params) {
+ Path hardLinkDir = params.tempDirCreator
+ .apply("context-" +
checksumForFileName(cacheKey.getContextDefinition()));
+ URL[] hardLinksAsURLs = new URL[params.paths.size()];
+ int i = 0;
+ for (Path p : params.paths) {
+ boolean reFetched;
+ Path hardLink = null;
+ do {
+ reFetched = false;
+ try {
+ hardLink = Files.createLink(hardLinkDir.resolve(p.getFileName()), p);
+ } catch (NoSuchFileException e) {
+ LOG.warn(
+ "Missing file {} while creating a hard link in {}; attempting
re-download of context resources",
+ p, hardLinkDir, e);
+ params.redownloader.accept(cacheKey.getContextDefinition());
Review Comment:
I had a few additional ideas here rather than re-download for the entire
context:
1. Maybe hard-link back to the resources directory for any hard-links
already created, so the downloader does have to re-download files where there
is a local copy already; this would also save space, because if the files
already hard-linked are re-downloaded, they will double their storage
utilization for the same files
2. Maybe delete the entire temporary directory, and discard any hard-links
already made; this forces a new downloaded copy, but at least it wouldn't
increase storage size, and it's a little simpler than the previous option
Also, I was thinking maybe it might be good to do the file verification for
each existing file, after doing the hard-links, rather than before. I don't
think this really matters, though. File corruption can occur after checking the
hard-links, too.
I was also thinking about simplifying the URLClassLoaderParams. I don't
think it needs the list of local paths, since it already has the
ContextDefinition and can derive the file names from that. Instead, it could
get a reference to the LocalStore instance, instead of the tempDir function and
the re-download consumer.
--
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]
