This is an automated email from the ASF dual-hosted git repository.
ctubbsii pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/accumulo-classloaders.git
The following commit(s) were added to refs/heads/main by this push:
new c0dfb1f More improvements for Java 21 (#74)
c0dfb1f is described below
commit c0dfb1fd405398e669ac6c5b92035d2d17f17a7f
Author: Christopher Tubbs <[email protected]>
AuthorDate: Wed Feb 18 14:50:34 2026 -0500
More improvements for Java 21 (#74)
Future-proof for Java 21:
* Avoid deprecated URL constructor
* Use URI instead of String in some places to make it easier to create a
URL when we need it
* Avoid Thread.getId (deprecated in Java 19), and just use the
Thread.toString (which includes the thread name and the thread ID)
* Use URI.resolve to construct HDFS URLs in tests rather than String
concatenation (must be used cautiously, due to HADOOP-19815, but still
better than String concatenation)
* Avoid deprecated exec method for launching umask by removing
unnecessary DFS config code (default behavior is sufficient for these)
* Put spotbugs and auto-service-annotations as optional compile time
dependency for every module (simplifies POMs) to ensure consistent
annotation processor execution
Also improve the build a bit more:
* Remove unused commons-vfs2-bom import dependency
* Improve concurrent delete test slightly to guarantee at least one
classloader per thread, and make test pass when it creates fewer
classloaders than we hope (it makes as many as it can in 10 seconds,
but it might not make as many as the 50 it was previously expecting)
---
modules/caching-class-loader/pom.xml | 12 ---
.../classloader/ccl/CachingClassLoaderFactory.java | 85 +++++++++++-----------
.../classloader/ccl/DeduplicationCacheKey.java | 7 +-
.../accumulo/classloader/ccl/LocalStore.java | 14 ++--
.../classloader/ccl/cli/CreateManifest.java | 3 +-
.../classloader/ccl/cli/InitializeCache.java | 4 +-
.../ccl/CachingClassLoaderFactoryTest.java | 67 ++++++++++-------
.../accumulo/classloader/ccl/LocalStoreTest.java | 11 +--
.../apache/accumulo/classloader/ccl/TestUtils.java | 33 ---------
.../accumulo/classloader/ccl/URLTypesTest.java | 8 +-
modules/hdfs-urlstreamhandler-provider/pom.xml | 12 ---
pom.xml | 24 +++---
12 files changed, 117 insertions(+), 163 deletions(-)
diff --git a/modules/caching-class-loader/pom.xml
b/modules/caching-class-loader/pom.xml
index 297f69e..581530a 100644
--- a/modules/caching-class-loader/pom.xml
+++ b/modules/caching-class-loader/pom.xml
@@ -30,18 +30,6 @@
<artifactId>caching-classloader</artifactId>
<name>classloader-extras-caching-classloader</name>
<dependencies>
- <dependency>
- <!-- needed for build checks, but not for runtime -->
- <groupId>com.github.spotbugs</groupId>
- <artifactId>spotbugs-annotations</artifactId>
- <optional>true</optional>
- </dependency>
- <dependency>
- <!-- needed for annotation processor during compile, but not after -->
- <groupId>com.google.auto.service</groupId>
- <artifactId>auto-service</artifactId>
- <optional>true</optional>
- </dependency>
<dependency>
<groupId>com.beust</groupId>
<artifactId>jcommander</artifactId>
diff --git
a/modules/caching-class-loader/src/main/java/org/apache/accumulo/classloader/ccl/CachingClassLoaderFactory.java
b/modules/caching-class-loader/src/main/java/org/apache/accumulo/classloader/ccl/CachingClassLoaderFactory.java
index 70db567..2697150 100644
---
a/modules/caching-class-loader/src/main/java/org/apache/accumulo/classloader/ccl/CachingClassLoaderFactory.java
+++
b/modules/caching-class-loader/src/main/java/org/apache/accumulo/classloader/ccl/CachingClassLoaderFactory.java
@@ -24,6 +24,7 @@ import java.io.IOException;
import java.io.UncheckedIOException;
import java.lang.ref.Cleaner;
import java.net.MalformedURLException;
+import java.net.URI;
import java.net.URL;
import java.net.URLClassLoader;
import java.nio.file.Path;
@@ -120,9 +121,9 @@ public class CachingClassLoaderFactory implements
ContextClassLoaderFactory {
return t;
});
- // stores the latest seen manifest for a remote URL; String types are used
here for the key
+ // stores the latest seen manifest for a remote URL; URI types are used here
for the key
// instead of URL because URL.hashCode could trigger network activity for
hostname lookups
- private final ConcurrentHashMap<String,Manifest> manifests = new
ConcurrentHashMap<>();
+ private final ConcurrentHashMap<URI,Manifest> manifests = new
ConcurrentHashMap<>();
// to keep this coherent with the manifests, updates to this should be done
in manifests.compute()
private final
DeduplicationCache<DeduplicationCacheKey,LocalStore,URLClassLoader>
classloaders =
@@ -131,7 +132,7 @@ public class CachingClassLoaderFactory implements
ContextClassLoaderFactory {
private final AtomicReference<LocalStore> localStore = new
AtomicReference<>();
- private final Map<String,Stopwatch> classloaderFailures = new HashMap<>();
+ private final Map<URI,Stopwatch> classloaderFailures = new HashMap<>();
private volatile Supplier<Duration> updateFailureGracePeriodMins;
// this is a BiConsumer so we can pass a type in the String
@@ -145,15 +146,15 @@ public class CachingClassLoaderFactory implements
ContextClassLoaderFactory {
* {@code #getClassLoader(String)} will recreate the manifests map entry and
schedule the monitor
* task.
*/
- private void monitor(final String url, long interval) {
- LOG.trace("Monitoring manifest {} for changes at {} second intervals",
url, interval);
+ private void monitor(final URI uri, long interval) {
+ LOG.trace("Monitoring manifest {} for changes at {} second intervals",
uri, interval);
executor.schedule(() -> {
try {
- checkMonitoredUrl(url, interval);
+ checkMonitoredUrl(uri, interval);
} catch (Throwable t) {
LOG.error("Unhandled exception occurred in manifest monitor thread.
Removing manifest {}.",
- url, t);
- manifests.remove(url);
+ uri, t);
+ manifests.remove(uri);
throw t;
}
}, interval, TimeUnit.SECONDS);
@@ -205,12 +206,12 @@ public class CachingClassLoaderFactory implements
ContextClassLoaderFactory {
public ClassLoader getClassLoader(final String manifestUrl) throws
ContextClassLoaderException {
Preconditions.checkState(localStore.get() != null,
"init not called before calling getClassLoader");
- requireNonNull(manifestUrl, "manifest URL must be supplied");
+ URI manifestURI = URI.create(requireNonNull(manifestUrl, "manifest URL
must be supplied"));
final var classloader = new AtomicReference<URLClassLoader>();
try {
// get the current manifest, or create it from the URL if absent; this
has the side effect of
// creating and caching a class loader instance if it doesn't exist for
the computed manifest
- manifests.compute(manifestUrl,
+ manifests.compute(manifestURI,
(key, previous) -> computeManifestAndClassLoader(classloader, key,
previous));
} catch (RuntimeException e) {
throw new ContextClassLoaderException(e.getMessage(), e);
@@ -219,15 +220,15 @@ public class CachingClassLoaderFactory implements
ContextClassLoaderFactory {
}
private Manifest
computeManifestAndClassLoader(AtomicReference<URLClassLoader> resultHolder,
- String url, Manifest previous) {
+ URI uri, Manifest previous) {
Manifest computed;
if (previous == null) {
try {
- computed = downloadManifest(url);
+ computed = downloadManifest(uri);
// we can set up monitoring now, but it will be blocked from doing
anything yet, until this
// finishes, since this code and the monitoring code both use
manifests.compute(), which
// is atomic/blocking for the same key
- monitor(url, computed.getMonitorIntervalSeconds());
+ monitor(uri, computed.getMonitorIntervalSeconds());
} catch (IOException e) {
throw new UncheckedIOException(e);
}
@@ -235,28 +236,28 @@ public class CachingClassLoaderFactory implements
ContextClassLoaderFactory {
computed = previous;
}
resultHolder.set(
- classloaders.computeIfAbsent(new DeduplicationCacheKey(url, computed),
localStore::get));
+ classloaders.computeIfAbsent(new DeduplicationCacheKey(uri, computed),
localStore::get));
return computed;
}
- private Manifest downloadManifest(String url) throws IOException {
- LOG.trace("Retrieving manifest from {}", url);
- URL urlUrl = new URL(url);
- allowedUrlChecker.accept("manifest", urlUrl);
- return Manifest.download(urlUrl);
+ private Manifest downloadManifest(URI uri) throws IOException {
+ LOG.trace("Retrieving manifest from {}", uri);
+ URL url = uri.toURL();
+ allowedUrlChecker.accept("manifest", url);
+ return Manifest.download(url);
}
- private void checkMonitoredUrl(String url, long interval) {
- Manifest current = manifests.compute(url, (key, previous) -> {
+ private void checkMonitoredUrl(URI uri, long interval) {
+ Manifest current = manifests.compute(uri, (key, previous) -> {
if (previous == null) {
// manifest has been removed from the map, no need to check for update
- LOG.debug("Manifest for {} not present, no longer monitoring for
changes", url);
+ LOG.debug("Manifest for {} not present, no longer monitoring for
changes", uri);
return null;
}
// check for any classloaders still in the cache that were created for a
manifest
// found at this URL
- if (!classloaders.anyMatch(cacheKey ->
cacheKey.getLocation().equals(url))) {
- LOG.debug("ClassLoader for {} not present, no longer monitoring for
changes", url);
+ if (!classloaders.anyMatch(cacheKey ->
cacheKey.getLocation().equals(uri))) {
+ LOG.debug("ClassLoader for {} not present, no longer monitoring for
changes", uri);
return null;
}
return previous;
@@ -266,57 +267,57 @@ public class CachingClassLoaderFactory implements
ContextClassLoaderFactory {
}
long nextInterval = interval;
try {
- final Manifest update = downloadManifest(url);
+ final Manifest update = downloadManifest(uri);
if (!current.getChecksum().equals(update.getChecksum())) {
- LOG.debug("Context manifest for {} has changed", url);
+ LOG.debug("Context manifest for {} has changed", uri);
localStore.get().storeContext(update);
- manifests.put(url, update);
+ manifests.put(uri, update);
nextInterval = update.getMonitorIntervalSeconds();
- classloaderFailures.remove(url);
+ classloaderFailures.remove(uri);
} else {
- LOG.trace("Context manifest for {} has not changed", url);
+ LOG.trace("Context manifest for {} has not changed", uri);
}
// reschedule this task to run if the manifest exists.
// Atomically lock on the key and only reschedule if the value is
present.
final long finalMonitorInterval = nextInterval;
- manifests.compute(url, (k, v) -> {
+ manifests.compute(uri, (k, v) -> {
if (v != null) {
- monitor(url, finalMonitorInterval);
+ monitor(uri, finalMonitorInterval);
}
return v;
});
} catch (IOException | RuntimeException e) {
- LOG.error("Error parsing updated manifest at {}. Classloader NOT
updated!", url, e);
- final Stopwatch failureTimer = classloaderFailures.get(url);
+ LOG.error("Error parsing updated manifest at {}. Classloader NOT
updated!", uri, e);
+ final Stopwatch failureTimer = classloaderFailures.get(uri);
var gracePeriod = updateFailureGracePeriodMins.get();
if (gracePeriod.isZero()) {
// failure monitoring is disabled
LOG.debug("Property {} not set, not tracking classloader failures for
{}",
- PROP_GRACE_PERIOD, url);
+ PROP_GRACE_PERIOD, uri);
} else if (failureTimer == null) {
// first failure, start the timer
- classloaderFailures.put(url, Stopwatch.createStarted());
+ classloaderFailures.put(uri, Stopwatch.createStarted());
LOG.debug(
"Tracking classloader failures for {}, will NOT return working
classloader if failures continue for {} minutes",
- url, gracePeriod.toMinutes());
+ uri, gracePeriod.toMinutes());
} else if (failureTimer.elapsed().compareTo(gracePeriod) > 0) {
// has been failing for the grace period
// unset the classloader reference so that the failure
// will return from getClassLoader in the calling thread
- LOG.info("Grace period for failing classloader has elapsed for {}",
url);
- manifests.remove(url);
- classloaderFailures.remove(url);
+ LOG.info("Grace period for failing classloader has elapsed for {}",
uri);
+ manifests.remove(uri);
+ classloaderFailures.remove(uri);
} else {
- LOG.trace("Failing to update classloader for {} within the grace
period", url, e);
+ LOG.trace("Failing to update classloader for {} within the grace
period", uri, e);
}
// reschedule this task to run if the manifest exists.
// Don't put this in finally block as we only want to reschedule
// on success or handled exception
// Atomically lock on the key and only reschedule if the value is
present.
final long finalMonitorInterval = nextInterval;
- manifests.compute(url, (k, v) -> {
+ manifests.compute(uri, (k, v) -> {
if (v != null) {
- monitor(url, finalMonitorInterval);
+ monitor(uri, finalMonitorInterval);
}
return v;
});
diff --git
a/modules/caching-class-loader/src/main/java/org/apache/accumulo/classloader/ccl/DeduplicationCacheKey.java
b/modules/caching-class-loader/src/main/java/org/apache/accumulo/classloader/ccl/DeduplicationCacheKey.java
index 1169368..2062a7a 100644
---
a/modules/caching-class-loader/src/main/java/org/apache/accumulo/classloader/ccl/DeduplicationCacheKey.java
+++
b/modules/caching-class-loader/src/main/java/org/apache/accumulo/classloader/ccl/DeduplicationCacheKey.java
@@ -21,20 +21,21 @@ package org.apache.accumulo.classloader.ccl;
import static java.util.Objects.hash;
import static java.util.Objects.requireNonNull;
+import java.net.URI;
import java.util.Objects;
import org.apache.accumulo.classloader.ccl.manifest.Manifest;
class DeduplicationCacheKey {
- private final String location;
+ private final URI location;
private final Manifest manifest;
- public DeduplicationCacheKey(String location, Manifest manifest) {
+ public DeduplicationCacheKey(URI location, Manifest manifest) {
this.location = requireNonNull(location);
this.manifest = requireNonNull(manifest);
}
- public String getLocation() {
+ public URI getLocation() {
return location;
}
diff --git
a/modules/caching-class-loader/src/main/java/org/apache/accumulo/classloader/ccl/LocalStore.java
b/modules/caching-class-loader/src/main/java/org/apache/accumulo/classloader/ccl/LocalStore.java
index bbda659..cce9c91 100644
---
a/modules/caching-class-loader/src/main/java/org/apache/accumulo/classloader/ccl/LocalStore.java
+++
b/modules/caching-class-loader/src/main/java/org/apache/accumulo/classloader/ccl/LocalStore.java
@@ -30,7 +30,7 @@ import java.io.Closeable;
import java.io.File;
import java.io.IOException;
import java.io.UncheckedIOException;
-import java.net.URISyntaxException;
+import java.net.URI;
import java.net.URL;
import java.net.URLClassLoader;
import java.nio.file.FileAlreadyExistsException;
@@ -108,12 +108,7 @@ public final class LocalStore {
private static Path baseDirStringToPath(final String value) {
if (value.startsWith("file:")) {
- try {
- return Path.of(new URL(value).toURI());
- } catch (IOException | URISyntaxException e) {
- throw new IllegalArgumentException(
- "Malformed file: URL specified for base directory: " + value, e);
- }
+ return Path.of(URI.create(value));
} else if (value.startsWith("/")) {
return Path.of(value);
}
@@ -343,8 +338,9 @@ public final class LocalStore {
}
if (t.isAlive()) {
- LOG.debug("Unexpectedly found download thread " + t.getId()
- + " still alive (thread was likely interrupted): " + t.getName());
+ LOG.debug(
+ "Unexpectedly found download thread still alive (thread was likely
interrupted): {}",
+ t);
}
}
}
diff --git
a/modules/caching-class-loader/src/main/java/org/apache/accumulo/classloader/ccl/cli/CreateManifest.java
b/modules/caching-class-loader/src/main/java/org/apache/accumulo/classloader/ccl/cli/CreateManifest.java
index 3441329..5d6e5a8 100644
---
a/modules/caching-class-loader/src/main/java/org/apache/accumulo/classloader/ccl/cli/CreateManifest.java
+++
b/modules/caching-class-loader/src/main/java/org/apache/accumulo/classloader/ccl/cli/CreateManifest.java
@@ -18,6 +18,7 @@
*/
package org.apache.accumulo.classloader.ccl.cli;
+import java.net.URI;
import java.net.URL;
import java.util.ArrayList;
import java.util.List;
@@ -66,7 +67,7 @@ public class CreateManifest implements KeywordExecutable {
URL[] urls = new URL[opts.files.size()];
int count = 0;
for (String f : opts.files) {
- urls[count++] = new URL(f);
+ urls[count++] = URI.create(f).toURL();
}
System.out.print(Manifest.create(opts.monitorInterval, opts.algorithm,
urls).toJson());
}
diff --git
a/modules/caching-class-loader/src/main/java/org/apache/accumulo/classloader/ccl/cli/InitializeCache.java
b/modules/caching-class-loader/src/main/java/org/apache/accumulo/classloader/ccl/cli/InitializeCache.java
index 903c502..da80125 100644
---
a/modules/caching-class-loader/src/main/java/org/apache/accumulo/classloader/ccl/cli/InitializeCache.java
+++
b/modules/caching-class-loader/src/main/java/org/apache/accumulo/classloader/ccl/cli/InitializeCache.java
@@ -18,7 +18,7 @@
*/
package org.apache.accumulo.classloader.ccl.cli;
-import java.net.URL;
+import java.net.URI;
import java.util.ArrayList;
import java.util.List;
@@ -69,7 +69,7 @@ public class InitializeCache implements KeywordExecutable {
opts.parseArgs(InitializeCache.class.getName(), args);
var localStore = new LocalStore(opts.directory, (a, b) -> {/* allow all
*/});
for (String manifestUrl : opts.manifests) {
- var manifest = Manifest.download(new URL(manifestUrl));
+ var manifest = Manifest.download(URI.create(manifestUrl).toURL());
localStore.storeContext(manifest);
if (opts.verify) {
var workingDir = localStore.createWorkingHardLinks(manifest, p -> {/*
do nothing */});
diff --git
a/modules/caching-class-loader/src/test/java/org/apache/accumulo/classloader/ccl/CachingClassLoaderFactoryTest.java
b/modules/caching-class-loader/src/test/java/org/apache/accumulo/classloader/ccl/CachingClassLoaderFactoryTest.java
index 83470a8..082d90c 100644
---
a/modules/caching-class-loader/src/test/java/org/apache/accumulo/classloader/ccl/CachingClassLoaderFactoryTest.java
+++
b/modules/caching-class-loader/src/test/java/org/apache/accumulo/classloader/ccl/CachingClassLoaderFactoryTest.java
@@ -43,6 +43,7 @@ import java.io.File;
import java.io.FileNotFoundException;
import java.io.UncheckedIOException;
import java.net.MalformedURLException;
+import java.net.URI;
import java.net.URL;
import java.nio.file.Files;
import java.nio.file.Path;
@@ -56,6 +57,7 @@ import java.util.concurrent.Executors;
import java.util.concurrent.Future;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicInteger;
import java.util.regex.PatternSyntaxException;
import java.util.stream.Collectors;
@@ -133,7 +135,7 @@ class CachingClassLoaderFactoryTest {
final var dst = new org.apache.hadoop.fs.Path("/contextB/TestB.jar");
fs.copyFromLocalFile(new
org.apache.hadoop.fs.Path(jarBOrigLocation.toURI()), dst);
assertTrue(fs.exists(dst));
- final URL jarBHdfsLocation = new URL(fs.getUri().toString() +
dst.toUri().toString());
+ final URL jarBHdfsLocation = fs.getUri().resolve(dst.toUri()).toURL();
// Have Jetty serve up files from Jar C directory
var jarCParentDirectory = Path.of(jarCOrigLocation.toURI()).getParent();
@@ -156,7 +158,7 @@ class CachingClassLoaderFactoryTest {
assertTrue(fs.exists(hdfsFile));
localAllUrl = localFile.toURI().toURL();
- hdfsAllUrl = new URL(fs.getUri().toString() + hdfsFile.toUri().toString());
+ hdfsAllUrl = fs.getUri().resolve(hdfsFile.toUri()).toURL();
jettyAllUrl = jetty.getURI().resolve("all.json").toURL();
classA = new TestClassInfo("test.TestObjectA", "Hello from A");
@@ -212,8 +214,8 @@ class CachingClassLoaderFactoryTest {
// case 3b: manifest URL matches, but resource URL fails to match the
pattern
// in this case, we use a new manifest, with a resource that doesn't exist
locally
var newResources = new LinkedHashSet<Resource>();
- var badUrl = "http://localhost/some/path";;
- newResources.add(new Resource(new URL(badUrl), "MD5", BAD_MD5));
+ var badUrl = URI.create("http://localhost/some/path";);
+ newResources.add(new Resource(badUrl.toURL(), "MD5", BAD_MD5));
var context2 = new Manifest(MONITOR_INTERVAL_SECS, newResources);
var disallowedContext =
tempDir.resolve("context-with-disallowed-resource-url.json");
Files.writeString(disallowedContext, context2.toJson());
@@ -278,16 +280,15 @@ class CachingClassLoaderFactoryTest {
public void testInvalidManifestURL() {
var ex =
assertThrows(ContextClassLoaderException.class, () ->
FACTORY.getClassLoader("/not/a/URL"));
- assertInstanceOf(UncheckedIOException.class, ex.getCause());
- assertInstanceOf(MalformedURLException.class, ex.getCause().getCause());
- assertEquals("no protocol: /not/a/URL",
ex.getCause().getCause().getMessage());
+ assertInstanceOf(IllegalArgumentException.class, ex.getCause());
+ assertEquals("URI is not absolute", ex.getCause().getMessage());
}
@Test
public void testInitialManifestEmpty() throws Exception {
// Create a new manifest file in HDFS, but with no content
final var manifest = createManifestFile(fs, "empty.json", null);
- final URL emptyUrl = new URL(fs.getUri().toString() +
manifest.toUri().toString());
+ final URL emptyUrl = fs.getUri().resolve(manifest.toUri()).toURL();
var ex = assertThrows(ContextClassLoaderException.class,
() -> FACTORY.getClassLoader(emptyUrl.toString()));
@@ -303,7 +304,7 @@ class CachingClassLoaderFactoryTest {
var manifest = Manifest.create(MONITOR_INTERVAL_SECS, "SHA-512",
jarAOrigLocation);
// write out invalid json
final var invalid = createManifestFile(fs, "invalid.json",
manifest.toJson().substring(0, 4));
- final URL invalidUrl = new URL(fs.getUri().toString() +
invalid.toUri().toString());
+ final URL invalidUrl = fs.getUri().resolve(invalid.toUri()).toURL();
var ex = assertThrows(ContextClassLoaderException.class,
() -> FACTORY.getClassLoader(invalidUrl.toString()));
@@ -315,7 +316,7 @@ class CachingClassLoaderFactoryTest {
public void testInitial() throws Exception {
var manifest = Manifest.create(MONITOR_INTERVAL_SECS, "SHA-512",
jarAOrigLocation);
final var initial = createManifestFile(fs, "initial.json",
manifest.toJson());
- final URL initialUrl = new URL(fs.getUri().toString() +
initial.toUri().toString());
+ final URL initialUrl = fs.getUri().resolve(initial.toUri()).toURL();
ClassLoader cl = FACTORY.getClassLoader(initialUrl.toString());
@@ -342,7 +343,7 @@ class CachingClassLoaderFactoryTest {
assertFalse(Files.exists(jarACopy));
final var initial = createManifestFile(fs, "missing-resource.json",
manifest.toJson());
- final URL initialUrl = new URL(fs.getUri().toString() +
initial.toUri().toString());
+ final URL initialUrl = fs.getUri().resolve(initial.toUri()).toURL();
var ex = assertThrows(ContextClassLoaderException.class,
() -> FACTORY.getClassLoader(initialUrl.toString()));
@@ -370,7 +371,7 @@ class CachingClassLoaderFactoryTest {
assertNotEquals(goodJson, badJson);
final var initial = createManifestFile(fs, "bad-resource-url.json",
badJson);
- final URL initialUrl = new URL(fs.getUri().toString() +
initial.toUri().toString());
+ final URL initialUrl = fs.getUri().resolve(initial.toUri()).toURL();
var ex = assertThrows(ContextClassLoaderException.class,
() -> FACTORY.getClassLoader(initialUrl.toString()));
@@ -391,7 +392,7 @@ class CachingClassLoaderFactoryTest {
var manifest = new Manifest(MONITOR_INTERVAL_SECS, resources);
final var initial = createManifestFile(fs, "bad-resource-checksum.json",
manifest.toJson());
- final URL initialUrl = new URL(fs.getUri().toString() +
initial.toUri().toString());
+ final URL initialUrl = fs.getUri().resolve(initial.toUri()).toURL();
var ex = assertThrows(ContextClassLoaderException.class,
() -> FACTORY.getClassLoader(initialUrl.toString()));
@@ -412,7 +413,7 @@ class CachingClassLoaderFactoryTest {
public void testUpdate() throws Exception {
final var manifest = Manifest.create(MONITOR_INTERVAL_SECS, "SHA-512",
jarAOrigLocation);
final var manifestPath = createManifestFile(fs, "update.json",
manifest.toJson());
- final URL manifestUrl = new URL(fs.getUri().toString() +
manifestPath.toUri().toString());
+ final URL manifestUrl = fs.getUri().resolve(manifestPath.toUri()).toURL();
final ClassLoader cl = FACTORY.getClassLoader(manifestUrl.toString());
@@ -442,7 +443,7 @@ class CachingClassLoaderFactoryTest {
public void testUpdateSameClassNameDifferentContent() throws Exception {
final var manifest = Manifest.create(MONITOR_INTERVAL_SECS, "SHA-512",
jarAOrigLocation);
final var manifestPath = createManifestFile(fs, "update-same-name.json",
manifest.toJson());
- final URL manifestUrl = new URL(fs.getUri().toString() +
manifestPath.toUri().toString());
+ final URL manifestUrl = fs.getUri().resolve(manifestPath.toUri()).toURL();
final ClassLoader cl = FACTORY.getClassLoader(manifestUrl.toString());
@@ -474,7 +475,7 @@ class CachingClassLoaderFactoryTest {
public void testUpdateManifestEmpty() throws Exception {
final var manifest = Manifest.create(MONITOR_INTERVAL_SECS, "SHA-512",
jarAOrigLocation);
final var manifestPath = createManifestFile(fs, "update-empty.json",
manifest.toJson());
- final URL manifestUrl = new URL(fs.getUri().toString() +
manifestPath.toUri().toString());
+ final URL manifestUrl = fs.getUri().resolve(manifestPath.toUri()).toURL();
final ClassLoader cl = FACTORY.getClassLoader(manifestUrl.toString());
@@ -505,7 +506,7 @@ class CachingClassLoaderFactoryTest {
final var manifest = Manifest.create(MONITOR_INTERVAL_SECS, "SHA-512",
jarAOrigLocation);
final var manifestPath =
createManifestFile(fs, "UpdateNonExistentResource.json",
manifest.toJson());
- final URL manifestUrl = new URL(fs.getUri().toString() +
manifestPath.toUri().toString());
+ final URL manifestUrl = fs.getUri().resolve(manifestPath.toUri()).toURL();
final ClassLoader cl = FACTORY.getClassLoader(manifestUrl.toString());
@@ -548,7 +549,7 @@ class CachingClassLoaderFactoryTest {
final var manifest = Manifest.create(MONITOR_INTERVAL_SECS, "SHA-512",
jarAOrigLocation);
final var manifestPath =
createManifestFile(fs, "UpdateBadResourceChecksum.json",
manifest.toJson());
- final URL manifestUrl = new URL(fs.getUri().toString() +
manifestPath.toUri().toString());
+ final URL manifestUrl = fs.getUri().resolve(manifestPath.toUri()).toURL();
final ClassLoader cl = FACTORY.getClassLoader(manifestUrl.toString());
@@ -583,7 +584,7 @@ class CachingClassLoaderFactoryTest {
final var manifest = Manifest.create(MONITOR_INTERVAL_SECS, "SHA-512",
jarAOrigLocation);
final var manifestPath =
createManifestFile(fs, "UpdateBadResourceChecksum.json",
manifest.toJson());
- final URL manifestUrl = new URL(fs.getUri().toString() +
manifestPath.toUri().toString());
+ final URL manifestUrl = fs.getUri().resolve(manifestPath.toUri()).toURL();
final ClassLoader cl = FACTORY.getClassLoader(manifestUrl.toString());
@@ -619,7 +620,7 @@ class CachingClassLoaderFactoryTest {
public void testUpdateInvalidJson() throws Exception {
final var manifest = Manifest.create(MONITOR_INTERVAL_SECS, "SHA-512",
jarAOrigLocation);
final var manifestPath = createManifestFile(fs, "update-invalid.json",
manifest.toJson());
- final URL updateUrl = new URL(fs.getUri().toString() +
manifestPath.toUri().toString());
+ final URL updateUrl = fs.getUri().resolve(manifestPath.toUri()).toURL();
final ClassLoader cl = FACTORY.getClassLoader(updateUrl.toString());
@@ -664,7 +665,7 @@ class CachingClassLoaderFactoryTest {
var manifest = Manifest.create(MONITOR_INTERVAL_SECS, "SHA-512",
jarAOrigLocation,
jarBOrigLocation, jarCOrigLocation, jarDOrigLocation);
final var manifestPath = createManifestFile(fs, "update-changing.json",
manifest.toJson());
- final URL manifestUrl = new URL(fs.getUri().toString() +
manifestPath.toUri().toString());
+ final URL manifestUrl = fs.getUri().resolve(manifestPath.toUri()).toURL();
final ClassLoader cl = FACTORY.getClassLoader(manifestUrl.toString());
testClassLoads(cl, classA);
@@ -735,7 +736,7 @@ class CachingClassLoaderFactoryTest {
final var manifest = Manifest.create(MONITOR_INTERVAL_SECS, "SHA-512",
jarAOrigLocation);
final var manifestPath =
createManifestFile(fs, "UpdateNonExistentResource.json",
manifest.toJson());
- final URL manifestUrl = new URL(fs.getUri().toString() +
manifestPath.toUri().toString());
+ final URL manifestUrl = fs.getUri().resolve(manifestPath.toUri()).toURL();
final ClassLoader cl = FACTORY.getClassLoader(manifestUrl.toString());
@@ -795,7 +796,7 @@ class CachingClassLoaderFactoryTest {
jarBOrigLocation, jarCOrigLocation, jarDOrigLocation);
final var manifestPath =
createManifestFile(fs, "update-external-modified.json",
manifest.toJson());
- final URL manifestUrl = new URL(fs.getUri().toString() +
manifestPath.toUri().toString());
+ final URL manifestUrl = fs.getUri().resolve(manifestPath.toUri()).toURL();
final ClassLoader cl = FACTORY.getClassLoader(manifestUrl.toString());
testClassLoads(cl, classA);
@@ -813,7 +814,7 @@ class CachingClassLoaderFactoryTest {
Files.copy(files.get(0), files.get(1), REPLACE_EXISTING);
final var update2 = createManifestFile(fs,
"update-external-modified2.json", manifest.toJson());
- final URL updateUrl2 = new URL(fs.getUri().toString() +
update2.toUri().toString());
+ final URL updateUrl2 = fs.getUri().resolve(update2.toUri()).toURL();
// The classloader should fail to create because one of the files in the
local filesystem cache
// has a checksum mismatch
@@ -853,6 +854,7 @@ class CachingClassLoaderFactoryTest {
List<Future<?>> futures = new ArrayList<>();
+ var numCreated = new AtomicInteger(0);
// create 10 threads that are continually creating new classloaders, the
deletes should cause
// hard link creations to fail sometimes
for (int i = 0; i < 10; i++) {
@@ -861,12 +863,13 @@ class CachingClassLoaderFactoryTest {
Timer timer = Timer.startNew();
int j = 0;
ClassLoader lastCl = null;
- while (!timer.hasElapsed(10, TimeUnit.SECONDS)) {
+ do {
var file = tempDir.resolve("context-cd-" + threadNum + "_" + j +
".json");
Files.writeString(file, manifest.toJson());
var url = file.toUri().toURL().toExternalForm();
final ClassLoader cl = FACTORY.getClassLoader(url);
+ numCreated.incrementAndGet();
// This test is assuming that each call above creates a new
classloader which in turn
// creates new hard links. This is checking that assumption in case
the impl changes and
// this test needs to be reevaluated.
@@ -877,7 +880,7 @@ class CachingClassLoaderFactoryTest {
testClassLoads(cl, classC);
testClassLoads(cl, classD);
j++;
- }
+ } while (!timer.hasElapsed(10, TimeUnit.SECONDS));
return null;
});
@@ -895,7 +898,15 @@ class CachingClassLoaderFactoryTest {
long workingDirsCount =
Files.list(baseCacheDir.resolve(WORKING_DIR)).filter(p ->
p.toFile().isDirectory()).count();
- // check that many hard link directories were created
- assertTrue(workingDirsCount > 50);
+ // check that many hard link directories were created; this is
non-deterministic; at least 50 is
+ // nice to see (5 per thread), but depending on performance, each thread
may not create them
+ // that quickly; we should get at least 1 per thread, though
+ assertTrue(workingDirsCount > 10,
+ "Expected at least 10 working dirs created (one per thread), but only
saw "
+ + workingDirsCount);
+ var numCls = numCreated.get();
+ assertTrue(workingDirsCount >= numCls,
+ "Expected the number of working directories to be at least the number
of classloaders created, but saw only "
+ + workingDirsCount + " dirs for " + numCls + " classloaders
created.");
}
}
diff --git
a/modules/caching-class-loader/src/test/java/org/apache/accumulo/classloader/ccl/LocalStoreTest.java
b/modules/caching-class-loader/src/test/java/org/apache/accumulo/classloader/ccl/LocalStoreTest.java
index ce050ed..ddf024f 100644
---
a/modules/caching-class-loader/src/test/java/org/apache/accumulo/classloader/ccl/LocalStoreTest.java
+++
b/modules/caching-class-loader/src/test/java/org/apache/accumulo/classloader/ccl/LocalStoreTest.java
@@ -34,6 +34,7 @@ import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.fail;
+import java.net.URI;
import java.net.URL;
import java.nio.file.Files;
import java.nio.file.Path;
@@ -91,7 +92,7 @@ public class LocalStoreTest {
final var dst = new org.apache.hadoop.fs.Path("/contextB/TestB.jar");
fs.copyFromLocalFile(new
org.apache.hadoop.fs.Path(jarBOrigLocation.toURI()), dst);
assertTrue(fs.exists(dst));
- final URL jarBNewLocation = new URL(fs.getUri().toString() +
dst.toUri().toString());
+ final URL jarBNewLocation = fs.getUri().resolve(dst.toUri()).toURL();
// Put C into Jetty
var jarCParentDirectory = Path.of(jarCOrigLocation.toURI()).getParent();
@@ -244,7 +245,7 @@ public class LocalStoreTest {
public void testClassLoader() throws Exception {
var localStore = new LocalStore(baseCacheDir, ALLOW_ALL_URLS);
localStore.storeContext(manifest);
- var cacheKey = new DeduplicationCacheKey("loc", manifest);
+ var cacheKey = new DeduplicationCacheKey(URI.create("loc"), manifest);
var cl = createClassLoader(cacheKey, localStore);
testClassLoads(cl, classA);
@@ -256,7 +257,7 @@ public class LocalStoreTest {
public void testClassLoaderUpdate() throws Exception {
var localStore = new LocalStore(baseCacheDir, ALLOW_ALL_URLS);
localStore.storeContext(manifest);
- var cacheKey = new DeduplicationCacheKey("loc", manifest);
+ var cacheKey = new DeduplicationCacheKey(URI.create("loc"), manifest);
final var cl = createClassLoader(cacheKey, localStore);
testClassLoads(cl, classA);
@@ -292,7 +293,7 @@ public class LocalStoreTest {
assertTrue(Files
.exists(baseCacheDir.resolve(RESOURCES_DIR).resolve(localResourceName(removedResource))));
- cacheKey = new DeduplicationCacheKey("loc", update);
+ cacheKey = new DeduplicationCacheKey(URI.create("loc"), update);
final var updatedCl = createClassLoader(cacheKey, localStore);
assertNotEquals(cl, updatedCl);
@@ -307,7 +308,7 @@ public class LocalStoreTest {
var localStore = new LocalStore(baseCacheDir, ALLOW_ALL_URLS);
assertEquals(0,
Files.list(localStore.workingDir()).filter(Files::isDirectory).count());
localStore.storeContext(manifest);
- var cacheKey = new DeduplicationCacheKey("loc", manifest);
+ var cacheKey = new DeduplicationCacheKey(URI.create("loc"), manifest);
assertEquals(0,
Files.list(localStore.workingDir()).filter(Files::isDirectory).count());
final var endBackgroundThread = new CountDownLatch(1);
diff --git
a/modules/caching-class-loader/src/test/java/org/apache/accumulo/classloader/ccl/TestUtils.java
b/modules/caching-class-loader/src/test/java/org/apache/accumulo/classloader/ccl/TestUtils.java
index 4cc2691..0a4af81 100644
---
a/modules/caching-class-loader/src/test/java/org/apache/accumulo/classloader/ccl/TestUtils.java
+++
b/modules/caching-class-loader/src/test/java/org/apache/accumulo/classloader/ccl/TestUtils.java
@@ -18,16 +18,13 @@
*/
package org.apache.accumulo.classloader.ccl;
-import static java.nio.charset.StandardCharsets.UTF_8;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
-import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStream;
-import java.io.InputStreamReader;
import java.net.URL;
import java.nio.file.Files;
import java.nio.file.Path;
@@ -111,33 +108,6 @@ public class TestUtils {
assertThrows(ClassNotFoundException.class, () ->
cl.loadClass(tci.getClassName()));
}
- private static String computeDatanodeDirectoryPermission() {
- // MiniDFSCluster will check the permissions on the data directories, but
does not
- // do a good job of setting them properly. We need to get the users umask
and set
- // the appropriate Hadoop property so that the data directories will be
created
- // with the correct permissions.
- try {
- Process p = Runtime.getRuntime().exec("/bin/sh -c umask");
- try (BufferedReader bri =
- new BufferedReader(new InputStreamReader(p.getInputStream(),
UTF_8))) {
- String line = bri.readLine();
- p.waitFor();
-
- if (line == null) {
- throw new IOException("umask input stream closed prematurely");
- }
- short umask = Short.parseShort(line.trim(), 8);
- // Need to set permission to 777 xor umask
- // leading zero makes java interpret as base 8
- int newPermission = 0777 ^ umask;
-
- return String.format("%03o", newPermission);
- }
- } catch (Exception e) {
- throw new RuntimeException("Error getting umask from O/S", e);
- }
- }
-
public static MiniDFSCluster getMiniCluster() throws IOException {
System.setProperty("java.io.tmpdir", System.getProperty("user.dir") +
"/target");
@@ -146,9 +116,6 @@ public class TestUtils {
// Setup HDFS
Configuration conf = new Configuration();
- conf.set("hadoop.security.token.service.use_ip", "true");
-
- conf.set("dfs.datanode.data.dir.perm",
computeDatanodeDirectoryPermission());
conf.setLong(DFSConfigKeys.DFS_BLOCK_SIZE_KEY, 1024 * 1024); // 1M
blocksize
MiniDFSCluster cluster = new MiniDFSCluster.Builder(conf).build();
diff --git
a/modules/caching-class-loader/src/test/java/org/apache/accumulo/classloader/ccl/URLTypesTest.java
b/modules/caching-class-loader/src/test/java/org/apache/accumulo/classloader/ccl/URLTypesTest.java
index c959eb0..cdf64b1 100644
---
a/modules/caching-class-loader/src/test/java/org/apache/accumulo/classloader/ccl/URLTypesTest.java
+++
b/modules/caching-class-loader/src/test/java/org/apache/accumulo/classloader/ccl/URLTypesTest.java
@@ -74,15 +74,15 @@ public class URLTypesTest {
var p = Path.of(jarPath.toURI());
final long origFileSize = TestUtils.getFileSize(p);
- MiniDFSCluster cluster = TestUtils.getMiniCluster();
+ MiniDFSCluster hdfs = TestUtils.getMiniCluster();
try {
- FileSystem fs = cluster.getFileSystem();
+ FileSystem fs = hdfs.getFileSystem();
assertTrue(fs.mkdirs(new org.apache.hadoop.fs.Path("/context1")));
var dst = new org.apache.hadoop.fs.Path("/context1/HelloWorld.jar");
fs.copyFromLocalFile(new org.apache.hadoop.fs.Path(jarPath.toURI()),
dst);
assertTrue(fs.exists(dst));
- URL fullPath = new URL(fs.getUri().toString() + dst.toUri().toString());
+ URL fullPath = fs.getUri().resolve(dst.toUri()).toURL();
LOG.info("Path to hdfs file: {}", fullPath);
assertEquals(origFileSize, TestUtils.getFileSize(fullPath));
@@ -90,7 +90,7 @@ public class URLTypesTest {
} catch (IOException e) {
throw new RuntimeException("Error setting up mini cluster", e);
} finally {
- cluster.shutdown();
+ hdfs.shutdown();
}
}
diff --git a/modules/hdfs-urlstreamhandler-provider/pom.xml
b/modules/hdfs-urlstreamhandler-provider/pom.xml
index fb6114d..9eb22f7 100644
--- a/modules/hdfs-urlstreamhandler-provider/pom.xml
+++ b/modules/hdfs-urlstreamhandler-provider/pom.xml
@@ -30,18 +30,6 @@
<artifactId>hdfs-urlstreamhandler-provider</artifactId>
<name>HDFS URLStreamHandlerProvider</name>
<dependencies>
- <dependency>
- <!-- needed for build checks, but not for runtime -->
- <groupId>com.github.spotbugs</groupId>
- <artifactId>spotbugs-annotations</artifactId>
- <optional>true</optional>
- </dependency>
- <dependency>
- <!-- needed for annotation processor during compile, but not after -->
- <groupId>com.google.auto.service</groupId>
- <artifactId>auto-service</artifactId>
- <optional>true</optional>
- </dependency>
<dependency>
<groupId>com.google.guava</groupId>
<artifactId>guava</artifactId>
diff --git a/pom.xml b/pom.xml
index 1d10c64..7cd42ae 100644
--- a/pom.xml
+++ b/pom.xml
@@ -130,6 +130,7 @@ under the License.
<rat.consoleOutput>true</rat.consoleOutput>
<sourceReleaseAssemblyDescriptor>source-release-tar</sourceReleaseAssemblyDescriptor>
<surefire.failIfNoSpecifiedTests>false</surefire.failIfNoSpecifiedTests>
+ <version.auto-service>1.1.1</version.auto-service>
</properties>
<dependencyManagement>
<dependencies>
@@ -142,12 +143,9 @@ under the License.
<scope>import</scope>
</dependency>
<dependency>
- <!-- no longer provided by accumulo after 2.1 -->
- <groupId>org.apache.commons</groupId>
- <artifactId>commons-vfs2-bom</artifactId>
- <version>2.10.0</version>
- <type>pom</type>
- <scope>import</scope>
+ <groupId>com.google.auto.service</groupId>
+ <artifactId>auto-service-annotations</artifactId>
+ <version>${version.auto-service}</version>
</dependency>
<dependency>
<groupId>org.apache.accumulo</groupId>
@@ -167,6 +165,12 @@ under the License.
<artifactId>spotbugs-annotations</artifactId>
<optional>true</optional>
</dependency>
+ <dependency>
+ <!-- needed for compile-time annotations only -->
+ <groupId>com.google.auto.service</groupId>
+ <artifactId>auto-service-annotations</artifactId>
+ <optional>true</optional>
+ </dependency>
</dependencies>
<build>
<pluginManagement>
@@ -380,13 +384,9 @@ under the License.
</goals>
<configuration>
<failOnWarning>true</failOnWarning>
- <ignoredUsedUndeclaredDependencies combine.children="append">
- <!-- auto-service-annotations is transitive via auto-service
-->
-
<undeclared>com.google.auto.service:auto-service-annotations:jar:*</undeclared>
- </ignoredUsedUndeclaredDependencies>
<ignoredUnusedDeclaredDependencies combine.children="append">
- <!-- auto-service used by the compiler for annotation
processing, not by code -->
- <unused>com.google.auto.service:auto-service:jar:*</unused>
+ <!-- auto-service annotations may or may not be used in each
module -->
+
<unused>com.google.auto.service:auto-service-annotations:jar:*</unused>
<!-- ignore false positive runtime dependencies -->
<unused>org.apache.logging.log4j:log4j-slf4j2-impl:*</unused>
<!-- spotbugs annotations may or may not be used in each
module -->
