Skip to content
Permalink
Browse files

Track deprecated property keys and warn users of their use

This change aims to expand on the current support we have for
annotated PropertyKeys. Currently, property keys are only
annotated as `@java.lang.Deprecated` and any extra info is
included within the generated javadoc. This however isn't
sufficient if we want to warn users at runtime about a key being
deprecated or removed.

This change adds two new custom annotations which allows
property keys to be marked as `@alluxio.conf.Deprecated` which
gives the ability to set a deprecation message. The other one is
`@alluxio.conf.Removed` which is similar to deprecated but
denotes PropertyKeys that have been completely removed from
use.

The messages within the annotations are provided to users via log
warning or an exception message if a key is marked as `@deprecated`
or `@removed`. When either annotation is provided on a key the
configuration validation step is responsible for logging a warning in
the case of `@deprecated` or throwing a RuntimeException for
`@removed`. Keys that reside in `RemovedKeys.java` are there for
historical purposes and shouldn't be used.

pr-link: #8616
change-id: cid-af3646499e8e3f589e448d83031160b3ffac05d9
  • Loading branch information...
ZacBlanco authored and alluxio-bot committed May 14, 2019
1 parent d95cdea commit be06317964c10716dea022fa5d109aafd86d4699
Showing with 564 additions and 333 deletions.
  1. +4 −4 core/client/hdfs/src/main/java/alluxio/hadoop/HadoopUtils.java
  2. +48 −0 core/common/src/main/java/alluxio/conf/Deprecated.java
  3. +131 −0 core/common/src/main/java/alluxio/conf/DeprecatedKeyChecker.java
  4. +6 −0 core/common/src/main/java/alluxio/conf/InstancedConfiguration.java
  5. +93 −220 core/common/src/main/java/alluxio/conf/PropertyKey.java
  6. +181 −0 core/common/src/main/java/alluxio/conf/RemovedKey.java
  7. +4 −6 core/common/src/main/java/alluxio/util/network/NetworkAddressUtils.java
  8. +0 −1 core/common/src/test/java/alluxio/ConfigurationTestUtils.java
  9. +49 −1 core/common/src/test/java/alluxio/conf/ConfigurationTest.java
  10. +13 −7 core/common/src/test/java/alluxio/{ → conf}/PropertyKeyTest.java
  11. +3 −2 core/common/src/test/java/alluxio/util/network/NetworkAddressUtilsTest.java
  12. +1 −5 core/server/common/src/main/java/alluxio/master/journalv0/JournalFormatter.java
  13. +3 −7 docs/_data/table/common-configuration.csv
  14. +4 −12 docs/_data/table/en/common-configuration.yml
  15. +10 −6 docs/_data/table/en/master-configuration.yml
  16. +3 −17 docs/_data/table/en/user-configuration.yml
  17. +1 −11 docs/_data/table/en/worker-configuration.yml
  18. +5 −3 docs/_data/table/master-configuration.csv
  19. +1 −8 docs/_data/table/user-configuration.csv
  20. +0 −5 docs/_data/table/worker-configuration.csv
  21. +0 −4 examples/src/main/java/alluxio/cli/JournalCrashTest.java
  22. +1 −2 tests/src/test/java/alluxio/client/fs/FileSystemMasterClientIntegrationTest.java
  23. +0 −1 tests/src/test/java/alluxio/server/ft/ZookeeperFailureIntegrationTest.java
  24. +1 −3 tests/src/test/java/alluxio/server/health/ProxyHealthCheckClientIntegrationTest.java
  25. +1 −3 underfs/swift/src/main/java/alluxio/underfs/swift/SwiftUnderFileSystem.java
  26. +1 −5 underfs/swift/src/main/java/alluxio/underfs/swift/SwiftUnderFileSystemFactory.java
@@ -146,10 +146,10 @@ public static String toStringHadoopInputSplit(InputSplit is) {
*/

public static void addSwiftCredentials(Configuration configuration) {
PropertyKey[] propertyNames = {PropertyKey.SWIFT_API_KEY, PropertyKey.SWIFT_TENANT_KEY,
PropertyKey.SWIFT_USER_KEY, PropertyKey.SWIFT_AUTH_URL_KEY,
PropertyKey.SWIFT_AUTH_METHOD_KEY, PropertyKey.SWIFT_PASSWORD_KEY,
PropertyKey.SWIFT_SIMULATION, PropertyKey.SWIFT_REGION_KEY};
PropertyKey[] propertyNames = { PropertyKey.SWIFT_TENANT_KEY, PropertyKey.SWIFT_USER_KEY,
PropertyKey.SWIFT_AUTH_URL_KEY, PropertyKey.SWIFT_AUTH_METHOD_KEY,
PropertyKey.SWIFT_PASSWORD_KEY, PropertyKey.SWIFT_SIMULATION,
PropertyKey.SWIFT_REGION_KEY };
setConfigurationFromSystemProperties(configuration, propertyNames);
}

@@ -0,0 +1,48 @@
/*
* The Alluxio Open Foundation licenses this work under the Apache License, version 2.0
* (the "License"). You may not use this work except in compliance with the License, which is
* available at www.apache.org/licenses/LICENSE-2.0
*
* This software is distributed on an "AS IS" basis, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND,
* either express or implied, as more fully set forth in the License.
*
* See the NOTICE file distributed with this work for information regarding copyright ownership.
*/

package alluxio.conf;

import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;

/**
* An annotation for PropertyKeys that can be used to mark a key as deprecated (slated for
* removal). If a key is marked with this annotation, then any Alluxio processes should trigger
* log warning messages upon configuration validation.
*
* This annotation is provided as an alternative to {@link java.lang.Deprecated}. Both
* annotations do not need to be specified on a key. The advantage of using this annotation is
* that we can supply a custom message rather than simply telling the user "This key has been
* deprecated".
*/
@Retention(RetentionPolicy.RUNTIME)
public @interface Deprecated {
/**
* A message to the user which could suggest an alternative key.
*
* The message should not mention anything about deprecation, but rather alternatives,
* side-effects, and/or the time-frame for removal of the key.
*
* An example may be:
*
* "This key is slated for removal in v2.0. An equivalent configuration property is
* alluxio.x.y.z."
*
* or
*
* "This key is slated for removal in v3.0. Afterwards, this value will no longer be
* configurable"
*
* @return A string explaining deprecation
*/
String message() default "";
}
@@ -0,0 +1,131 @@
/*
* The Alluxio Open Foundation licenses this work under the Apache License, version 2.0
* (the "License"). You may not use this work except in compliance with the License, which is
* available at www.apache.org/licenses/LICENSE-2.0
*
* This software is distributed on an "AS IS" basis, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND,
* either express or implied, as more fully set forth in the License.
*
* See the NOTICE file distributed with this work for information regarding copyright ownership.
*/

package alluxio.conf;

import java.lang.reflect.Field;
import java.util.HashMap;
import java.util.Map;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.regex.Matcher;

import javax.annotation.Nullable;

/**
* This annotation checker should be used to determine whether a {@link PropertyKey} or
* {@link alluxio.conf.PropertyKey.Template} has a given annotation.
*
* The class is mainly useful to check for {@link Deprecated} annotations on property keys and
* return the associated message, if any.
*/
public class DeprecatedKeyChecker {
private static final Class<PropertyKey.Template> TEMPLATE_CLASS =
PropertyKey.Template.class;
private static final Class<PropertyKey> KEY_CLASS = PropertyKey.class;

private Map<PropertyKey, Deprecated> mAnnotatedKeys;
private Map<PropertyKey.Template, Deprecated> mAnnotatedTemplates;
private AtomicBoolean mInitialized = new AtomicBoolean(false);

/**
* Create a new instance of {@link DeprecatedKeyChecker}.
*/
public DeprecatedKeyChecker() { }

/**
* Given a class to search, a field type, and an annotation type will return a map of all
* fields which are marked with the given annotation to the instance of the annotation.
*
* @param searchType the class to search through of the given type
* @param <T> The type of the field to retrieve
* @return a map of all fields within {@code searchType} class of the type T that are annotated
* with the {@link Deprecated} annotation
*/
private static <T> Map<T, Deprecated> populateAnnotatedKeyMap(Class<T> searchType) {
Map<T, Deprecated> annotations = new HashMap<>();
// Iterate over all fields in the class
for (Field field : searchType.getDeclaredFields()) {
// If the field isn't equal to the class type, skip it
if (!field.getType().equals(searchType)) {
continue;
}

Deprecated keyAnnotation = field.getAnnotation(Deprecated.class);

try {
// Field#get parameter can be null if retrieving a static field (all PKs are static)
// See https://docs.oracle.com/javase/8/docs/api/java/lang/reflect/Field.html
// This also works with Template enums
T key = searchType.cast(field.get(null));
if (keyAnnotation != null) {
annotations.put(key, keyAnnotation);
}
} catch (IllegalAccessException e) {
throw new RuntimeException(e);
}
}
return annotations;
}

private Deprecated getKeyAnnotation(PropertyKey key) {
if (!mInitialized.get()) {
initialize();
}
if (mAnnotatedKeys.containsKey(key)) {
return mAnnotatedKeys.get(key);
} else {
for (Map.Entry<PropertyKey.Template, Deprecated> e : mAnnotatedTemplates.entrySet()) {
Matcher match = e.getKey().match(key.getName());
if (match.matches()) {
return e.getValue();
}
}
}
return null;
}

private void initialize() {
if (!mInitialized.getAndSet(true)) {
mAnnotatedKeys =
populateAnnotatedKeyMap(KEY_CLASS);
mAnnotatedTemplates =
populateAnnotatedKeyMap(TEMPLATE_CLASS);
}
}

/**
* Returns whether or not the given property key is marked by this class' annotation.
*
* It first checks if the specific key has the annotation, otherwise it will fall back to checking
* if the key's name matches any of the PropertyKey templates. If no keys or templates match, it
* will return false. This will only return true when the key is marked with a {@link Deprecated}
* annotation.
*
* @param key the property key to check
* @return if this property key is deprecated
* @see Deprecated
* @see PropertyKey#getDeprecationMessage(PropertyKey)
*/
public boolean hasAnnotation(PropertyKey key) {
return getKeyAnnotation(key) != null;
}

/**
* Returns the annotation attached to the given property key.
*
* @param key the key to get the annotation for
* @return the annotation if it exists, null otherwise
*/
@Nullable
public Deprecated getAnnotation(PropertyKey key) {
return getKeyAnnotation(key);
}
}
@@ -354,7 +354,13 @@ public void validate() {
+ "and must be specified as a JVM property. "
+ "If no JVM property is present, Alluxio will use default value '%s'.",
key.getName(), key.getDefaultValue());

if (PropertyKey.isDeprecated(key) && getSource(key).compareTo(Source.DEFAULT) != 0) {
LOG.warn("{} is deprecated. Please avoid using this key in the future. {}", key.getName(),
PropertyKey.getDeprecationMessage(key));
}
}

checkTimeouts();
checkWorkerPorts();
checkUserFileBufferBytes();

0 comments on commit be06317

Please sign in to comment.
You can’t perform that action at this time.