Skip to content
Permalink
Browse files
Minor little cleanups (#2695)
* Remove or narrow unnecessary or broad warnings suppressions
* Shorten the lookup of annotations in Property.java
* Use Streams where they simplify or improve the processing of big loops
* Shorten a few lines with casts using var
* Improve use of generics in a few places
* Fix license header in proto generation to be consistent with project
* Avoid unnecessary reflection to load DistCp in ShellServerIT
* Remove unneeded super()
  • Loading branch information
ctubbsii committed May 13, 2022
1 parent 79fb796 commit 1a0bb1e3d58583ab1f9a90dd1bbc33138345d85a
Showing 54 changed files with 170 additions and 273 deletions.
@@ -76,9 +76,7 @@ public class BloomFilter extends Filter {
BitSet bits;

/** Default constructor - use with readFields */
public BloomFilter() {
super();
}
public BloomFilter() {}

/**
* Constructor
@@ -32,7 +32,6 @@
import org.apache.accumulo.core.spi.common.ContextClassLoaderFactory;
import org.apache.accumulo.core.util.threads.ThreadPools;
import org.apache.accumulo.core.util.threads.Threads;
import org.apache.accumulo.start.classloader.vfs.AccumuloVFSClassLoader;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

@@ -43,24 +42,33 @@
* for all requested contexts. This class is used internally to Accumulo only, and should not be
* used by users in their configuration.
*/
@SuppressWarnings({"deprecation", "removal"})
public class DefaultContextClassLoaderFactory implements ContextClassLoaderFactory {

private static final AtomicBoolean isInstantiated = new AtomicBoolean(false);
private static final Logger LOG = LoggerFactory.getLogger(DefaultContextClassLoaderFactory.class);
private static final String className = DefaultContextClassLoaderFactory.class.getName();

@SuppressWarnings("removal")
private static final Property VFS_CONTEXT_CLASSPATH_PROPERTY =
Property.VFS_CONTEXT_CLASSPATH_PROPERTY;

public DefaultContextClassLoaderFactory(final AccumuloConfiguration accConf) {
if (!isInstantiated.compareAndSet(false, true)) {
throw new IllegalStateException("Can only instantiate " + className + " once");
}
Supplier<Map<String,String>> contextConfigSupplier =
() -> accConf.getAllPropertiesWithPrefix(Property.VFS_CONTEXT_CLASSPATH_PROPERTY);
AccumuloVFSClassLoader.setContextConfig(contextConfigSupplier);
() -> accConf.getAllPropertiesWithPrefix(VFS_CONTEXT_CLASSPATH_PROPERTY);
setContextConfig(contextConfigSupplier);
LOG.debug("ContextManager configuration set");
startCleanupThread(accConf, contextConfigSupplier);
}

@SuppressWarnings("deprecation")
private static void setContextConfig(Supplier<Map<String,String>> contextConfigSupplier) {
org.apache.accumulo.start.classloader.vfs.AccumuloVFSClassLoader
.setContextConfig(contextConfigSupplier);
}

private static void startCleanupThread(final AccumuloConfiguration conf,
final Supplier<Map<String,String>> contextConfigSupplier) {
ScheduledFuture<?> future = ThreadPools.getClientThreadPools((t, e) -> {
@@ -69,18 +77,26 @@ private static void startCleanupThread(final AccumuloConfiguration conf,
.scheduleWithFixedDelay(Threads.createNamedRunnable(className + "-cleanup", () -> {
LOG.trace("{}-cleanup thread, properties: {}", className, conf);
Set<String> contextsInUse = contextConfigSupplier.get().keySet().stream()
.map(p -> p.substring(Property.VFS_CONTEXT_CLASSPATH_PROPERTY.getKey().length()))
.map(p -> p.substring(VFS_CONTEXT_CLASSPATH_PROPERTY.getKey().length()))
.collect(Collectors.toSet());
LOG.trace("{}-cleanup thread, contexts in use: {}", className, contextsInUse);
AccumuloVFSClassLoader.removeUnusedContexts(contextsInUse);
removeUnusedContexts(contextsInUse);
}), 1, 1, MINUTES);
ThreadPools.watchNonCriticalScheduledTask(future);
LOG.debug("Context cleanup timer started at 60s intervals");
}

@SuppressWarnings("deprecation")
private static void removeUnusedContexts(Set<String> contextsInUse) {
org.apache.accumulo.start.classloader.vfs.AccumuloVFSClassLoader
.removeUnusedContexts(contextsInUse);
}

@SuppressWarnings("deprecation")
@Override
public ClassLoader getClassLoader(String contextName) {
return AccumuloVFSClassLoader.getContextClassLoader(contextName);
return org.apache.accumulo.start.classloader.vfs.AccumuloVFSClassLoader
.getContextClassLoader(contextName);
}

}
@@ -34,9 +34,7 @@ public SampleNotPresentException(String message) {
super(message);
}

public SampleNotPresentException() {
super();
}
public SampleNotPresentException() {}

private static final long serialVersionUID = 1L;

@@ -37,9 +37,7 @@
public class RangeInputSplit extends org.apache.accumulo.core.client.mapreduce.RangeInputSplit
implements InputSplit {

public RangeInputSplit() {
super();
}
public RangeInputSplit() {}

public RangeInputSplit(RangeInputSplit split) throws IOException {
super(split);
@@ -94,16 +94,13 @@ public static <T extends AuthenticationToken> T deserialize(Class<T> tokenType,
* @see #serialize(AuthenticationToken)
*/
public static AuthenticationToken deserialize(String tokenClassName, byte[] tokenBytes) {
Class<? extends AuthenticationToken> tokenType = null;
try {
@SuppressWarnings("unchecked")
Class<? extends AuthenticationToken> tmpTokenType =
(Class<? extends AuthenticationToken>) Class.forName(tokenClassName);
tokenType = tmpTokenType;
var tokenType = (Class<? extends AuthenticationToken>) Class.forName(tokenClassName);
return deserialize(tokenType, tokenBytes);
} catch (ClassNotFoundException e) {
throw new IllegalArgumentException("Class not available " + tokenClassName, e);
}
return deserialize(tokenType, tokenBytes);
}

/**
@@ -38,9 +38,7 @@ public class CredentialProviderToken extends PasswordToken {
private String name;
private String credentialProviders;

public CredentialProviderToken() {
super();
}
public CredentialProviderToken() {}

public CredentialProviderToken(String name, String credentialProviders) throws IOException {
requireNonNull(name);
@@ -45,7 +45,6 @@ public class DelegationTokenImpl extends PasswordToken implements DelegationToke
private final AuthenticationTokenIdentifier identifier;

public DelegationTokenImpl() {
super();
this.identifier = new AuthenticationTokenIdentifier(new TAuthenticationTokenIdentifier());
}

@@ -29,7 +29,5 @@ public IsolationException(Exception cause) {
super(cause);
}

public IsolationException() {
super();
}
public IsolationException() {}
}
@@ -39,9 +39,7 @@
public class BatchInputSplit extends org.apache.accumulo.core.clientImpl.mapreduce.BatchInputSplit
implements InputSplit {

public BatchInputSplit() {
super();
}
public BatchInputSplit() {}

public BatchInputSplit(BatchInputSplit split) throws IOException {
super(split);
@@ -19,9 +19,11 @@
package org.apache.accumulo.core.conf;

import java.lang.annotation.Annotation;
import java.util.Arrays;
import java.util.EnumSet;
import java.util.HashMap;
import java.util.HashSet;
import java.util.function.Predicate;

import org.apache.accumulo.core.Constants;
import org.apache.accumulo.core.classloader.ClassLoaderUtil;
@@ -39,7 +41,6 @@
import org.apache.accumulo.core.spi.scan.SimpleScanDispatcher;
import org.apache.accumulo.core.util.format.DefaultFormatter;
import org.apache.accumulo.core.util.interpret.DefaultScanInterpreter;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import com.google.common.base.Preconditions;
@@ -1550,21 +1551,17 @@ private void precomputeAnnotations() {
hasAnnotation(Sensitive.class) || hasPrefixWithAnnotation(getKey(), Sensitive.class);
isDeprecated =
hasAnnotation(Deprecated.class) || hasPrefixWithAnnotation(getKey(), Deprecated.class);
if (hasAnnotation(Deprecated.class)) {
Deprecated dep = getAnnotation(Deprecated.class);
if (dep != null) {
deprecatedSince = dep.since();
}
Deprecated dep = getAnnotation(Deprecated.class);
if (dep != null) {
deprecatedSince = dep.since();
}
isExperimental =
hasAnnotation(Experimental.class) || hasPrefixWithAnnotation(getKey(), Experimental.class);
isReplaced =
hasAnnotation(ReplacedBy.class) || hasPrefixWithAnnotation(getKey(), ReplacedBy.class);
if (hasAnnotation(ReplacedBy.class)) {
ReplacedBy rb = getAnnotation(ReplacedBy.class);
if (rb != null) {
replacedBy = rb.property();
}
ReplacedBy rb = getAnnotation(ReplacedBy.class);
if (rb != null) {
replacedBy = rb.property();
}
annotationsComputed = true;
}
@@ -1581,76 +1578,35 @@ public static boolean isSensitive(String key) {
Property prop = propertiesByKey.get(key);
if (prop != null) {
return prop.isSensitive();
} else {
for (String prefix : validPrefixes) {
if (key.startsWith(prefix)) {
if (propertiesByKey.get(prefix).isSensitive()) {
return true;
}
}
}
}
return false;
return validPrefixes.stream().filter(key::startsWith).map(propertiesByKey::get)
.anyMatch(Property::isSensitive);
}

private <T extends Annotation> boolean hasAnnotation(Class<T> annotationType) {
Logger log = LoggerFactory.getLogger(getClass());
try {
for (Annotation a : getClass().getField(name()).getAnnotations()) {
if (annotationType.isInstance(a)) {
return true;
}
}
} catch (SecurityException | NoSuchFieldException e) {
log.error("{}", e.getMessage(), e);
}
return false;
return getAnnotation(annotationType) != null;
}

private <T extends Annotation> T getAnnotation(Class<T> annotationType) {
Logger log = LoggerFactory.getLogger(getClass());
try {
for (Annotation a : getClass().getField(name()).getAnnotations()) {
if (annotationType.isInstance(a)) {
@SuppressWarnings("unchecked")
T uncheckedA = (T) a;
return uncheckedA;
}
}
return getClass().getField(name()).getAnnotation(annotationType);
} catch (SecurityException | NoSuchFieldException e) {
log.error("{}", e.getMessage(), e);
LoggerFactory.getLogger(getClass()).error("{}", e.getMessage(), e);
}
return null;
}

private static <T extends Annotation> boolean hasPrefixWithAnnotation(String key,
Class<T> annotationType) {
for (String prefix : validPrefixes) {
if (key.startsWith(prefix)) {
if (propertiesByKey.get(prefix).hasAnnotation(annotationType)) {
return true;
}
}
}

return false;
Predicate<Property> hasIt = prop -> prop.hasAnnotation(annotationType);
return validPrefixes.stream().filter(key::startsWith).map(propertiesByKey::get).anyMatch(hasIt);
}

private static final HashSet<String> validTableProperties = new HashSet<>();
private static final HashSet<String> validProperties = new HashSet<>();
private static final HashSet<String> validPrefixes = new HashSet<>();
private static final HashMap<String,Property> propertiesByKey = new HashMap<>();

private static boolean isKeyValidlyPrefixed(String key) {
for (String prefix : validPrefixes) {
if (key.startsWith(prefix)) {
return true;
}
}

return false;
}

/**
* Checks if the given property and value are valid. A property is valid if the property key is
* valid see {@link #isValidTablePropertyKey} and that the value is a valid format for the type
@@ -1676,7 +1632,8 @@ public static boolean isTablePropertyValid(final String key, final String value)
* @return true if key is valid (recognized, or has a recognized prefix)
*/
public static boolean isValidPropertyKey(String key) {
return validProperties.contains(key) || isKeyValidlyPrefixed(key);
return validProperties.contains(key) || validPrefixes.stream().anyMatch(key::startsWith);

}

/**
@@ -1828,24 +1785,24 @@ public static <T> T createInstanceFromPropertyName(AccumuloConfiguration conf, P
// Precomputing information here avoids :
// * Computing it each time a method is called
// * Using synch to compute the first time a method is called
for (Property p : Property.values()) {
propertiesByKey.put(p.getKey(), p);
if (p.getType().equals(PropertyType.PREFIX)) {
validPrefixes.add(p.getKey());
} else {
validProperties.add(p.getKey());
}
// exclude prefix types (prevents setting a prefix type like table.custom or
// table.constraint, directly, since they aren't valid properties on their own)
if (!p.getType().equals(PropertyType.PREFIX)
&& p.getKey().startsWith(Property.TABLE_PREFIX.getKey())) {
validTableProperties.add(p.getKey());
}
}
Predicate<Property> isPrefix = p -> p.getType() == PropertyType.PREFIX;
Arrays.stream(Property.values())
// record all properties by key
.peek(p -> propertiesByKey.put(p.getKey(), p))
// save all the prefix properties
.peek(p -> {
if (isPrefix.test(p))
validPrefixes.add(p.getKey());
})
// only use the keys for the non-prefix properties from here on
.filter(isPrefix.negate()).map(Property::getKey)
// everything left is a valid property
.peek(validProperties::add)
// but some are also valid table properties
.filter(k -> k.startsWith(Property.TABLE_PREFIX.getKey()))
.forEach(validTableProperties::add);

// order is very important here the following code relies on the maps and sets populated above
for (Property p : Property.values()) {
p.precomputeAnnotations();
}
Arrays.stream(Property.values()).forEach(Property::precomputeAnnotations);
}
}
@@ -40,7 +40,7 @@ public class CachedBlock implements HeapSize, Comparable<CachedBlock> {
ClassSize.align(ClassSize.OBJECT + (3 * ClassSize.REFERENCE) + (2 * SizeConstants.SIZEOF_LONG)
+ ClassSize.STRING + ClassSize.BYTE_BUFFER + ClassSize.REFERENCE);

public static enum BlockPriority {
public enum BlockPriority {
/**
* Accessed a single time (used for scan-resistance)
*/
@@ -135,7 +135,6 @@ public class LruBlockCache extends SynchronousLoadingBlockCache implements Block
@SuppressFBWarnings(value = "SC_START_IN_CTOR",
justification = "bad practice to start threads in constructor; probably needs rewrite")
public LruBlockCache(final LruBlockCacheConfiguration conf) {
super();
this.conf = conf;

int mapInitialSize = (int) Math.ceil(1.2 * conf.getMaxSize() / conf.getBlockSize());
@@ -304,7 +304,6 @@ private abstract class BaseBlockLoader implements Loader {
private boolean loadingMetaBlock;

public BaseBlockLoader(boolean loadingMetaBlock) {
super();
this.loadingMetaBlock = loadingMetaBlock;
}

@@ -151,9 +151,8 @@ public String description() {
protected Class<? extends BiFunction<Key,Value,String>> getFormatter(String formatterClazz)
throws ClassNotFoundException {
@SuppressWarnings("unchecked")
Class<? extends BiFunction<Key,Value,String>> clazz =
(Class<? extends BiFunction<Key,Value,String>>) this.getClass().getClassLoader()
.loadClass(formatterClazz).asSubclass(BiFunction.class);
var clazz = (Class<? extends BiFunction<Key,Value,String>>) this.getClass().getClassLoader()
.loadClass(formatterClazz).asSubclass(BiFunction.class);
return clazz;
}

@@ -23,8 +23,9 @@
/**
* Exception - Meta Block with the same name already exists.
*/
@SuppressWarnings("serial")
public class MetaBlockAlreadyExists extends IOException {
private static final long serialVersionUID = -6797037044124244666L;

/**
* Constructor
*

0 comments on commit 1a0bb1e

Please sign in to comment.