xichen01 commented on code in PR #4774:
URL: https://github.com/apache/ozone/pull/4774#discussion_r1206455943
##########
hadoop-hdds/config/src/main/java/org/apache/hadoop/hdds/conf/ConfigurationReflectionUtil.java:
##########
@@ -34,99 +36,101 @@ public final class ConfigurationReflectionUtil {
private ConfigurationReflectionUtil() {
}
+ public static <T> Map<String, Field> mapReconfigurableProperties(
+ Class<T> configurationClass) {
+ Optional<String> prefix = getPrefix(configurationClass);
+ Map<String, Field> props =
+ mapReconfigurableProperties(configurationClass, prefix);
+ Class<? super T> superClass = configurationClass.getSuperclass();
+ while (superClass != null) {
+ props.putAll(mapReconfigurableProperties(superClass, prefix));
+ superClass = superClass.getSuperclass();
+ }
+ return props;
+ }
+
+ private static <T> Map<String, Field> mapReconfigurableProperties(
+ Class<T> configurationClass, Optional<String> prefix) {
+ Map<String, Field> props = new HashMap<>();
+ for (Field field : configurationClass.getDeclaredFields()) {
+ if (field.isAnnotationPresent(Config.class)) {
+ Config configAnnotation = field.getAnnotation(Config.class);
+
+ if (configAnnotation.reconfigurable()) {
+ checkNotFinal(configurationClass, field);
+ props.put(getFullKey(prefix, configAnnotation), field);
+ }
+ }
+ }
+ return props;
+ }
+
public static <T> void injectConfiguration(
ConfigurationSource configuration,
Class<T> configurationClass,
- T configObject, String prefix) {
+ T configObject, String prefix, boolean reconfiguration) {
injectConfigurationToObject(configuration, configurationClass,
configObject,
- prefix);
+ prefix, reconfiguration);
Class<? super T> superClass = configurationClass.getSuperclass();
while (superClass != null) {
injectConfigurationToObject(configuration, superClass, configObject,
- prefix);
+ prefix, reconfiguration);
superClass = superClass.getSuperclass();
}
}
- public static <T> void injectConfigurationToObject(ConfigurationSource from,
+ private static <T> void injectConfigurationToObject(ConfigurationSource from,
Class<T> configurationClass,
T configuration,
- String prefix) {
+ String prefix,
+ boolean reconfiguration
+ ) {
for (Field field : configurationClass.getDeclaredFields()) {
if (field.isAnnotationPresent(Config.class)) {
- if ((field.getModifiers() & Modifier.FINAL) != 0) {
- throw new ConfigurationException(String.format(
- "Trying to set final field %s#%s, probably indicates misplaced "
+
- "@Config annotation",
- configurationClass.getSimpleName(), field.getName()));
- }
-
- String fieldLocation =
- configurationClass + "." + field.getName();
+ checkNotFinal(configurationClass, field);
Config configAnnotation = field.getAnnotation(Config.class);
- String key = prefix + "." + configAnnotation.key();
+ if (reconfiguration && !configAnnotation.reconfigurable()) {
+ continue;
+ }
+ String key = prefix + "." + configAnnotation.key();
String defaultValue = configAnnotation.defaultValue();
+ String value = from.get(key, defaultValue);
Review Comment:
If the `value` is equal with `defaultValue`, maybe we can skip `setField()`
when reconfiguration.
And we can output a LOG when the reconfiguration is successful.
##########
hadoop-hdds/config/src/main/java/org/apache/hadoop/hdds/conf/ConfigurationReflectionUtil.java:
##########
@@ -34,99 +36,101 @@ public final class ConfigurationReflectionUtil {
private ConfigurationReflectionUtil() {
}
+ public static <T> Map<String, Field> mapReconfigurableProperties(
+ Class<T> configurationClass) {
+ Optional<String> prefix = getPrefix(configurationClass);
+ Map<String, Field> props =
+ mapReconfigurableProperties(configurationClass, prefix);
+ Class<? super T> superClass = configurationClass.getSuperclass();
+ while (superClass != null) {
+ props.putAll(mapReconfigurableProperties(superClass, prefix));
+ superClass = superClass.getSuperclass();
+ }
+ return props;
+ }
+
+ private static <T> Map<String, Field> mapReconfigurableProperties(
+ Class<T> configurationClass, Optional<String> prefix) {
+ Map<String, Field> props = new HashMap<>();
+ for (Field field : configurationClass.getDeclaredFields()) {
+ if (field.isAnnotationPresent(Config.class)) {
+ Config configAnnotation = field.getAnnotation(Config.class);
+
+ if (configAnnotation.reconfigurable()) {
+ checkNotFinal(configurationClass, field);
+ props.put(getFullKey(prefix, configAnnotation), field);
+ }
+ }
+ }
+ return props;
+ }
+
public static <T> void injectConfiguration(
ConfigurationSource configuration,
Class<T> configurationClass,
- T configObject, String prefix) {
+ T configObject, String prefix, boolean reconfiguration) {
injectConfigurationToObject(configuration, configurationClass,
configObject,
- prefix);
+ prefix, reconfiguration);
Class<? super T> superClass = configurationClass.getSuperclass();
while (superClass != null) {
injectConfigurationToObject(configuration, superClass, configObject,
- prefix);
+ prefix, reconfiguration);
superClass = superClass.getSuperclass();
}
}
- public static <T> void injectConfigurationToObject(ConfigurationSource from,
+ private static <T> void injectConfigurationToObject(ConfigurationSource from,
Class<T> configurationClass,
T configuration,
- String prefix) {
+ String prefix,
+ boolean reconfiguration
+ ) {
for (Field field : configurationClass.getDeclaredFields()) {
if (field.isAnnotationPresent(Config.class)) {
- if ((field.getModifiers() & Modifier.FINAL) != 0) {
- throw new ConfigurationException(String.format(
- "Trying to set final field %s#%s, probably indicates misplaced "
+
- "@Config annotation",
- configurationClass.getSimpleName(), field.getName()));
- }
-
- String fieldLocation =
- configurationClass + "." + field.getName();
+ checkNotFinal(configurationClass, field);
Config configAnnotation = field.getAnnotation(Config.class);
- String key = prefix + "." + configAnnotation.key();
+ if (reconfiguration && !configAnnotation.reconfigurable()) {
+ continue;
+ }
+ String key = prefix + "." + configAnnotation.key();
String defaultValue = configAnnotation.defaultValue();
+ String value = from.get(key, defaultValue);
- ConfigType type = configAnnotation.type();
+ setField(configurationClass, configuration, field, configAnnotation,
Review Comment:
Is there a thread safety issue here? Reading the configuration may be in
progress at any time while the program is running.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
