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.git
The following commit(s) were added to refs/heads/main by this push: new 48679fe Improvements to loading of compression configs (#2538) 48679fe is described below commit 48679fef73e246de52fbeecad03f974f2116b97a Author: Christopher Tubbs <ctubb...@apache.org> AuthorDate: Wed Mar 2 10:19:30 2022 -0500 Improvements to loading of compression configs (#2538) This follows up from the work done in #2518 to remove the use of the Java ServiceLoader for built-in known compression configs. Instead of loading all compression configs through the ServiceLoader, this change loads the known ones, and combines those with those discovered by the ServiceLoader. This prevents issues during development where the integration tests cannot be run inside an IDE, like Eclipse, due to the auto-service annotation processor not having been executed in that environment to make the implementing classes known to the ServiceLoader. Now, they are always available for integration tests. This also prevents the `@AutoService` annotation from being exposed in SPI classes, although it's not clear that's necessarily a problem. Additionally, this cleans up some of the previous code, that unnecessarily placed the supported algorithm names in a list, then converted it to an array, only to have it converted to a list again. For the pom, this change also sets the auto-service version in a property, so the multiple uses will use the same version, and explicitly includes the annotation processor in the maven-compiler-plugin configuration so it is consistent both within the errorprone profile and outside it. And finally, revert the version of Hadoop to 3.3.0, because 3.3.1 seems to be the cause of some flaky testing. In particular, it seems to cause unexpected EOF errors when trying to talk to data nodes, which is revealed by regular CountNameNodeOpsBulkIT failures, some so bad they seem to corrupt the XML output from failsafe, causing failsafe to be unable to read the result of the test. I was not able to reproduce these failures using 3.3.0, whereas I was seeing them regularly with 3.3.1. --- .../accumulo/core/file/rfile/CreateEmpty.java | 8 ++--- .../core/file/rfile/bcfile/Compression.java | 35 ++++++++++++++-------- .../core/spi/file/rfile/compression/Bzip2.java | 3 -- .../core/spi/file/rfile/compression/Gz.java | 3 -- .../core/spi/file/rfile/compression/Lz4.java | 3 -- .../core/spi/file/rfile/compression/Lzo.java | 3 -- .../spi/file/rfile/compression/NoCompression.java | 3 -- .../core/spi/file/rfile/compression/Snappy.java | 3 -- .../core/spi/file/rfile/compression/ZStandard.java | 3 -- pom.xml | 18 ++++++----- 10 files changed, 36 insertions(+), 46 deletions(-) diff --git a/core/src/main/java/org/apache/accumulo/core/file/rfile/CreateEmpty.java b/core/src/main/java/org/apache/accumulo/core/file/rfile/CreateEmpty.java index 2f82260..975f4c1 100644 --- a/core/src/main/java/org/apache/accumulo/core/file/rfile/CreateEmpty.java +++ b/core/src/main/java/org/apache/accumulo/core/file/rfile/CreateEmpty.java @@ -19,7 +19,6 @@ package org.apache.accumulo.core.file.rfile; import java.util.ArrayList; -import java.util.Arrays; import java.util.List; import org.apache.accumulo.core.cli.Help; @@ -59,10 +58,9 @@ public class CreateEmpty implements KeywordExecutable { public static class IsSupportedCompressionAlgorithm implements IParameterValidator { @Override public void validate(String name, String value) throws ParameterException { - String[] algorithms = Compression.getSupportedAlgorithms(); - if (!Arrays.asList(algorithms).contains(value)) { - throw new ParameterException( - "Compression codec must be one of " + Arrays.toString(algorithms)); + List<String> algorithms = Compression.getSupportedAlgorithms(); + if (!algorithms.contains(value)) { + throw new ParameterException("Compression codec must be one of " + algorithms); } } } diff --git a/core/src/main/java/org/apache/accumulo/core/file/rfile/bcfile/Compression.java b/core/src/main/java/org/apache/accumulo/core/file/rfile/bcfile/Compression.java index 8b73e6b..3e9ac64 100644 --- a/core/src/main/java/org/apache/accumulo/core/file/rfile/bcfile/Compression.java +++ b/core/src/main/java/org/apache/accumulo/core/file/rfile/bcfile/Compression.java @@ -18,13 +18,23 @@ */ package org.apache.accumulo.core.file.rfile.bcfile; -import java.util.ArrayList; +import java.util.List; import java.util.Map; import java.util.ServiceLoader; +import java.util.Set; +import java.util.function.Function; import java.util.stream.Collectors; +import java.util.stream.Stream; import java.util.stream.StreamSupport; +import org.apache.accumulo.core.spi.file.rfile.compression.Bzip2; import org.apache.accumulo.core.spi.file.rfile.compression.CompressionAlgorithmConfiguration; +import org.apache.accumulo.core.spi.file.rfile.compression.Gz; +import org.apache.accumulo.core.spi.file.rfile.compression.Lz4; +import org.apache.accumulo.core.spi.file.rfile.compression.Lzo; +import org.apache.accumulo.core.spi.file.rfile.compression.NoCompression; +import org.apache.accumulo.core.spi.file.rfile.compression.Snappy; +import org.apache.accumulo.core.spi.file.rfile.compression.ZStandard; import org.apache.hadoop.conf.Configuration; /** @@ -43,22 +53,21 @@ public final class Compression { // Configuration object. protected static final Configuration conf = new Configuration(); - private static final ServiceLoader<CompressionAlgorithmConfiguration> COMPRESSION_ALGORITHMS = + private static final ServiceLoader<CompressionAlgorithmConfiguration> FOUND_ALGOS = ServiceLoader.load(CompressionAlgorithmConfiguration.class); - private static final Map<String,CompressionAlgorithm> CONFIGURED_ALGORITHMS = - StreamSupport.stream(COMPRESSION_ALGORITHMS.spliterator(), false) + private static final Set<CompressionAlgorithmConfiguration> BUILTIN_ALGOS = Set.of(new Gz(), + new Bzip2(), new Lz4(), new Lzo(), new NoCompression(), new Snappy(), new ZStandard()); + + private static final Map<String, + CompressionAlgorithm> CONFIGURED_ALGORITHMS = Stream + .concat(BUILTIN_ALGOS.stream(), StreamSupport.stream(FOUND_ALGOS.spliterator(), false)) .map(a -> new CompressionAlgorithm(a, conf)) - .collect(Collectors.toMap(algo -> algo.getName(), algo -> algo)); + .collect(Collectors.toMap(CompressionAlgorithm::getName, Function.identity())); - public static String[] getSupportedAlgorithms() { - ArrayList<String> supportedAlgorithms = new ArrayList<>(); - CONFIGURED_ALGORITHMS.forEach((k, v) -> { - if (v.isSupported()) { - supportedAlgorithms.add(k); - } - }); - return supportedAlgorithms.toArray(new String[0]); + public static List<String> getSupportedAlgorithms() { + return CONFIGURED_ALGORITHMS.entrySet().stream().filter(e -> e.getValue().isSupported()) + .map(Map.Entry::getKey).collect(Collectors.toList()); } public static CompressionAlgorithm getCompressionAlgorithmByName(final String name) { diff --git a/core/src/main/java/org/apache/accumulo/core/spi/file/rfile/compression/Bzip2.java b/core/src/main/java/org/apache/accumulo/core/spi/file/rfile/compression/Bzip2.java index 8f53f97..62b1ca3 100644 --- a/core/src/main/java/org/apache/accumulo/core/spi/file/rfile/compression/Bzip2.java +++ b/core/src/main/java/org/apache/accumulo/core/spi/file/rfile/compression/Bzip2.java @@ -18,9 +18,6 @@ */ package org.apache.accumulo.core.spi.file.rfile.compression; -import com.google.auto.service.AutoService; - -@AutoService(CompressionAlgorithmConfiguration.class) public class Bzip2 implements CompressionAlgorithmConfiguration { @Override diff --git a/core/src/main/java/org/apache/accumulo/core/spi/file/rfile/compression/Gz.java b/core/src/main/java/org/apache/accumulo/core/spi/file/rfile/compression/Gz.java index 220eb00..7296d7c 100644 --- a/core/src/main/java/org/apache/accumulo/core/spi/file/rfile/compression/Gz.java +++ b/core/src/main/java/org/apache/accumulo/core/spi/file/rfile/compression/Gz.java @@ -18,9 +18,6 @@ */ package org.apache.accumulo.core.spi.file.rfile.compression; -import com.google.auto.service.AutoService; - -@AutoService(CompressionAlgorithmConfiguration.class) public class Gz implements CompressionAlgorithmConfiguration { @Override diff --git a/core/src/main/java/org/apache/accumulo/core/spi/file/rfile/compression/Lz4.java b/core/src/main/java/org/apache/accumulo/core/spi/file/rfile/compression/Lz4.java index 7ee111c..60f8e6a 100644 --- a/core/src/main/java/org/apache/accumulo/core/spi/file/rfile/compression/Lz4.java +++ b/core/src/main/java/org/apache/accumulo/core/spi/file/rfile/compression/Lz4.java @@ -18,9 +18,6 @@ */ package org.apache.accumulo.core.spi.file.rfile.compression; -import com.google.auto.service.AutoService; - -@AutoService(CompressionAlgorithmConfiguration.class) public class Lz4 implements CompressionAlgorithmConfiguration { @Override diff --git a/core/src/main/java/org/apache/accumulo/core/spi/file/rfile/compression/Lzo.java b/core/src/main/java/org/apache/accumulo/core/spi/file/rfile/compression/Lzo.java index ebacaa9..fc819f5 100644 --- a/core/src/main/java/org/apache/accumulo/core/spi/file/rfile/compression/Lzo.java +++ b/core/src/main/java/org/apache/accumulo/core/spi/file/rfile/compression/Lzo.java @@ -18,9 +18,6 @@ */ package org.apache.accumulo.core.spi.file.rfile.compression; -import com.google.auto.service.AutoService; - -@AutoService(CompressionAlgorithmConfiguration.class) public class Lzo implements CompressionAlgorithmConfiguration { @Override diff --git a/core/src/main/java/org/apache/accumulo/core/spi/file/rfile/compression/NoCompression.java b/core/src/main/java/org/apache/accumulo/core/spi/file/rfile/compression/NoCompression.java index fcbae48..ea7e704 100644 --- a/core/src/main/java/org/apache/accumulo/core/spi/file/rfile/compression/NoCompression.java +++ b/core/src/main/java/org/apache/accumulo/core/spi/file/rfile/compression/NoCompression.java @@ -20,9 +20,6 @@ package org.apache.accumulo.core.spi.file.rfile.compression; import org.apache.accumulo.core.file.rfile.bcfile.IdentityCodec; -import com.google.auto.service.AutoService; - -@AutoService(CompressionAlgorithmConfiguration.class) public class NoCompression implements CompressionAlgorithmConfiguration { @Override diff --git a/core/src/main/java/org/apache/accumulo/core/spi/file/rfile/compression/Snappy.java b/core/src/main/java/org/apache/accumulo/core/spi/file/rfile/compression/Snappy.java index 78e3738..b626f8d 100644 --- a/core/src/main/java/org/apache/accumulo/core/spi/file/rfile/compression/Snappy.java +++ b/core/src/main/java/org/apache/accumulo/core/spi/file/rfile/compression/Snappy.java @@ -18,9 +18,6 @@ */ package org.apache.accumulo.core.spi.file.rfile.compression; -import com.google.auto.service.AutoService; - -@AutoService(CompressionAlgorithmConfiguration.class) public class Snappy implements CompressionAlgorithmConfiguration { @Override diff --git a/core/src/main/java/org/apache/accumulo/core/spi/file/rfile/compression/ZStandard.java b/core/src/main/java/org/apache/accumulo/core/spi/file/rfile/compression/ZStandard.java index 2ef1542..f29c911 100644 --- a/core/src/main/java/org/apache/accumulo/core/spi/file/rfile/compression/ZStandard.java +++ b/core/src/main/java/org/apache/accumulo/core/spi/file/rfile/compression/ZStandard.java @@ -18,9 +18,6 @@ */ package org.apache.accumulo.core.spi.file.rfile.compression; -import com.google.auto.service.AutoService; - -@AutoService(CompressionAlgorithmConfiguration.class) public class ZStandard implements CompressionAlgorithmConfiguration { @Override diff --git a/pom.xml b/pom.xml index f082116..363e64e 100644 --- a/pom.xml +++ b/pom.xml @@ -116,6 +116,7 @@ <properties> <!-- used for filtering the java source with the current version --> <accumulo.release.version>${project.version}</accumulo.release.version> + <auto-service.version>1.0.1</auto-service.version> <!-- bouncycastle version for test dependencies --> <bouncycastle.version>1.70</bouncycastle.version> <!-- Curator version --> @@ -130,7 +131,8 @@ <failsafe.forkCount>1</failsafe.forkCount> <failsafe.groups /> <failsafe.reuseForks>false</failsafe.reuseForks> - <hadoop.version>3.3.1</hadoop.version> + <!-- Version 3.3.1 seems to have issues especially with CountNameNodeOptsBulkIT --> + <hadoop.version>3.3.0</hadoop.version> <htrace.hadoop.version>4.1.0-incubating</htrace.hadoop.version> <it.failIfNoSpecifiedTests>false</it.failIfNoSpecifiedTests> <!-- prevent introduction of new compiler warnings --> @@ -263,7 +265,7 @@ <dependency> <groupId>com.google.auto.service</groupId> <artifactId>auto-service</artifactId> - <version>1.0.1</version> + <version>${auto-service.version}</version> </dependency> <dependency> <groupId>com.google.code.findbugs</groupId> @@ -805,6 +807,13 @@ <arg>-Xmaxwarns</arg> <arg>5</arg> </compilerArgs> + <annotationProcessorPaths combine.children="append"> + <path> + <groupId>com.google.auto.service</groupId> + <artifactId>auto-service</artifactId> + <version>${auto-service.version}</version> + </path> + </annotationProcessorPaths> </configuration> </plugin> <plugin> @@ -1698,11 +1707,6 @@ <artifactId>error_prone_core</artifactId> <version>${errorprone.version}</version> </path> - <path> - <groupId>com.google.auto.service</groupId> - <artifactId>auto-service</artifactId> - <version>1.0</version> - </path> </annotationProcessorPaths> </configuration> </plugin>