HDDS-2661. Use field based Config annotation instead of method based

hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/XceiverClientManager.java

@@ -297,40 +297,50 @@ public static synchronized XceiverClientMetrics getXceiverClientMetrics() {
   @ConfigGroup(prefix = "scm.container.client")
   public static class ScmClientConfig {
 
+    @Config(key = "max.size",
+        defaultValue = "256",
+        tags = {OZONE, PERFORMANCE},
+        description =
+            "Controls the maximum number of connections that are cached via"
+                + " client connection pooling. If the number of connections"
+                + " exceed this count, then the oldest idle connection is "
+                + "evicted."
+    )
     private int maxSize;
-    private long staleThreshold;
-    private int maxOutstandingRequests;
-
-    public long getStaleThreshold(TimeUnit unit) {
-      return unit.convert(staleThreshold, MILLISECONDS);
-    }
 
     @Config(key = "idle.threshold",
         type = ConfigType.TIME, timeUnit = MILLISECONDS,
         defaultValue = "10s",
-        tags = { OZONE, PERFORMANCE },
+        tags = {OZONE, PERFORMANCE},
         description =
             "In the standalone pipelines, the SCM clients use netty to "
-            + " communicate with the container. It also uses connection pooling"
-            + " to reduce client side overheads. This allows a connection to"
-            + " stay idle for a while before the connection is closed."
+                + " communicate with the container. It also uses connection "
+                + "pooling"
+                + " to reduce client side overheads. This allows a connection"
+                + " to"
+                + " stay idle for a while before the connection is closed."
+    )
+    private long staleThreshold;
+
+    @Config(key = "max.outstanding.requests",
+        defaultValue = "100",
+        tags = {OZONE, PERFORMANCE},
+        description =
+            "Controls the maximum number of outstanding async requests that can"
+                + " be handled by the Standalone as well as Ratis client."
     )
-    public void setStaleThreshold(long staleThreshold) {
-      this.staleThreshold = staleThreshold;
+    private int maxOutstandingRequests;
+
+    public long getStaleThreshold(TimeUnit unit) {
+      return unit.convert(staleThreshold, MILLISECONDS);
     }
 
+
     public int getMaxSize() {
       return maxSize;
     }
 
-    @Config(key = "max.size",
-        defaultValue = "256",
-        tags = { OZONE, PERFORMANCE },
-        description =
-            "Controls the maximum number of connections that are cached via"
-            + " client connection pooling. If the number of connections"
-            + " exceed this count, then the oldest idle connection is evicted."
-    )
+    @VisibleForTesting
     public void setMaxSize(int maxSize) {
       this.maxSize = maxSize;
     }
@@ -339,16 +349,6 @@ public int getMaxOutstandingRequests() {
       return maxOutstandingRequests;
     }
 
-    @Config(key = "max.outstanding.requests",
-        defaultValue = "100",
-        tags = { OZONE, PERFORMANCE },
-        description =
-            "Controls the maximum number of outstanding async requests that can"
-            + " be handled by the Standalone as well as Ratis client."
-    )
-    public void setMaxOutstandingRequests(int maxOutstandingRequests) {
-      this.maxOutstandingRequests = maxOutstandingRequests;
-    }
   }
 
 }

hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/conf/OzoneConfiguration.java

@@ -18,6 +18,7 @@
 
 package org.apache.hadoop.hdds.conf;
 
+import javax.annotation.PostConstruct;
 import javax.xml.bind.JAXBContext;
 import javax.xml.bind.JAXBException;
 import javax.xml.bind.Unmarshaller;
@@ -26,6 +27,7 @@
 import javax.xml.bind.annotation.XmlElement;
 import javax.xml.bind.annotation.XmlRootElement;
 import java.io.IOException;
+import java.lang.reflect.Field;
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
 import java.net.URL;
@@ -103,77 +105,126 @@ public List<Property> readPropertyFromXml(URL url) throws JAXBException {
    */
   public <T> T getObject(Class<T> configurationClass) {
 
-    T configuration;
+    T configObject;
 
     try {
-      configuration = configurationClass.newInstance();
+      configObject = configurationClass.newInstance();
     } catch (InstantiationException | IllegalAccessException e) {
       throw new ConfigurationException(
           "Configuration class can't be created: " + configurationClass, e);
     }
     ConfigGroup configGroup =
         configurationClass.getAnnotation(ConfigGroup.class);
+
     String prefix = configGroup.prefix();
 
-    for (Method setterMethod : configurationClass.getMethods()) {
-      if (setterMethod.isAnnotationPresent(Config.class)) {
+    injectConfiguration(configurationClass, configObject, prefix);
 
-        String methodLocation =
-            configurationClass + "." + setterMethod.getName();
+    callPostConstruct(configurationClass, configObject);
 
-        Config configAnnotation = setterMethod.getAnnotation(Config.class);
+    return configObject;
 
-        String key = prefix + "." + configAnnotation.key();
+  }
 
-        Class<?>[] parameterTypes = setterMethod.getParameterTypes();
-        if (parameterTypes.length != 1) {
-          throw new ConfigurationException(
-              "@Config annotation should be used on simple setter: "
-                  + methodLocation);
+  private <T> void injectConfiguration(Class<T> configurationClass,
+      T configObject, String prefix) {
+    injectConfigurationToObject(configurationClass, configObject, prefix);
+    Class<? super T> superClass = configurationClass.getSuperclass();
+    while (superClass != null) {
+      injectConfigurationToObject(superClass, configObject, prefix);
+      superClass = superClass.getSuperclass();
+    }
+  }
+
+  private <T> void callPostConstruct(Class<T> configurationClass,
+      T configObject) {
+    for (Method method : configurationClass.getMethods()) {
+      if (method.isAnnotationPresent(PostConstruct.class)) {
+        try {
+          method.invoke(configObject);
+        } catch (IllegalAccessException ex) {
+          throw new IllegalArgumentException(
+              "@PostConstruct method in " + configurationClass
+                  + " is not accessible");
+        } catch (InvocationTargetException e) {
+          if (e.getCause() instanceof RuntimeException) {
+            throw (RuntimeException) e.getCause();
+          } else {
+            throw new IllegalArgumentException(
+                "@PostConstruct can't be executed on " + configurationClass
+                    + " after configObject "
+                    + "injection", e);
+          }
         }
+      }
+    }
+  }
+
+  private <T> void injectConfigurationToObject(Class<T> configurationClass,
+      T configuration, String prefix) {
+    for (Field field : configurationClass.getDeclaredFields()) {
+      if (field.isAnnotationPresent(Config.class)) {
+
+        String fieldLocation =
+            configurationClass + "." + field.getName();
+
+        Config configAnnotation = field.getAnnotation(Config.class);
+
+        String key = prefix + "." + configAnnotation.key();
 
         ConfigType type = configAnnotation.type();
 
         if (type == ConfigType.AUTO) {
-          type = detectConfigType(parameterTypes[0], methodLocation);
+          type = detectConfigType(field.getType(), fieldLocation);
         }
 
         //Note: default value is handled by ozone-default.xml. Here we can
         //use any default.
         try {
           switch (type) {
           case STRING:
-            setterMethod.invoke(configuration, get(key));
+            forcedFieldSet(field, configuration, get(key));
             break;
           case INT:
-            setterMethod.invoke(configuration,
-                getInt(key, 0));
+            forcedFieldSet(field, configuration, getInt(key, 0));
             break;
           case BOOLEAN:
-            setterMethod.invoke(configuration,
-                getBoolean(key, false));
+            forcedFieldSet(field, configuration, getBoolean(key, false));
             break;
           case LONG:
-            setterMethod.invoke(configuration,
-                getLong(key, 0));
+            forcedFieldSet(field, configuration, getLong(key, 0));
             break;
           case TIME:
-            setterMethod.invoke(configuration,
+            forcedFieldSet(field, configuration,
                 getTimeDuration(key, 0, configAnnotation.timeUnit()));
             break;
           default:
             throw new ConfigurationException(
-                "Unsupported ConfigType " + type + " on " + methodLocation);
+                "Unsupported ConfigType " + type + " on " + fieldLocation);
           }
-        } catch (InvocationTargetException | IllegalAccessException e) {
+        } catch (IllegalAccessException e) {
           throw new ConfigurationException(
-              "Can't inject configuration to " + methodLocation, e);
+              "Can't inject configuration to " + fieldLocation, e);
         }
 
       }
     }
-    return configuration;
+  }
 
+  /**
+   * Set the value of one field even if it's private.
+   */
+  private <T> void forcedFieldSet(Field field, T object, Object value)
+      throws IllegalAccessException {
+    boolean accessChanged = false;
+    if (!field.isAccessible()) {
+      field.setAccessible(true);
+      accessChanged = true;
+    }
+    field.set(object, value);
+    if (accessChanged) {
+      field.setAccessible(false);
+    }
   }
 
   private ConfigType detectConfigType(Class<?> parameterType,

hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/ScmConfig.java

@@ -27,26 +27,30 @@
  */
 @ConfigGroup(prefix = "hdds.scm")
 public class ScmConfig {
-  private String principal;
-  private String keytab;
 
   @Config(key = "kerberos.principal",
-        type = ConfigType.STRING,
-        defaultValue = "",
-        tags = { ConfigTag.SECURITY, ConfigTag.OZONE },
-        description = "This Kerberos principal is used by the SCM service."
+      type = ConfigType.STRING,
+      defaultValue = "",
+      tags = { ConfigTag.SECURITY, ConfigTag.OZONE },
+      description = "This Kerberos principal is used by the SCM service."
+  )
+  private String principal;
+
+  @Config(key = "kerberos.keytab.file",
+      type = ConfigType.STRING,
+      defaultValue = "",
+      tags = { ConfigTag.SECURITY, ConfigTag.OZONE },
+      description = "The keytab file used by SCM daemon to login as "+
+          "its service principal."
   )
+  private String keytab;
+
+
   public void setKerberosPrincipal(String kerberosPrincipal) {
     this.principal = kerberosPrincipal;
   }
 
-  @Config(key = "kerberos.keytab.file",
-        type = ConfigType.STRING,
-        defaultValue = "",
-        tags = { ConfigTag.SECURITY, ConfigTag.OZONE },
-        description = "The keytab file used by SCM daemon to login as "+
-              "its service principal."
-  )
+
   public void setKerberosKeytab(String kerberosKeytab) {
     this.keytab = kerberosKeytab;
   }

hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/utils/db/RocksDBConfiguration.java

@@ -29,13 +29,29 @@
 @ConfigGroup(prefix = "hadoop.hdds.db")
 public class RocksDBConfiguration {
 
-  private boolean rocksdbLogEnabled;
-
   @Config(key = "rocksdb.logging.enabled",
       type = ConfigType.BOOLEAN,
       defaultValue = "false",
       tags = {ConfigTag.OM},
       description = "Enable/Disable RocksDB logging for OM.")
+  private boolean rocksdbLogEnabled;
+
+  @Config(key = "rocksdb.logging.level",
+      type = ConfigType.STRING,
+      defaultValue = "INFO",
+      tags = {ConfigTag.OM},
+      description = "OM RocksDB logging level (INFO/DEBUG/WARN/ERROR/FATAL)")
+  private String rocksdbLogLevel;
+
+  @Config(key = "rocksdb.writeoption.sync",
+      type = ConfigType.BOOLEAN,
+      defaultValue = "false",
+      tags = {ConfigTag.OM},
+      description = "Enable/Disable Sync option. If true write will be " +
+          "considered complete, once flushed to persistent storage. If false," +
+          " writes are flushed asynchronously.")
+  private boolean syncOption;
+
   public void setRocksdbLoggingEnabled(boolean enabled) {
     this.rocksdbLogEnabled = enabled;
   }
@@ -44,13 +60,6 @@ public boolean isRocksdbLoggingEnabled() {
     return rocksdbLogEnabled;
   }
 
-  private String rocksdbLogLevel;
-
-  @Config(key = "rocksdb.logging.level",
-      type = ConfigType.STRING,
-      defaultValue = "INFO",
-      tags = {ConfigTag.OM},
-      description = "OM RocksDB logging level (INFO/DEBUG/WARN/ERROR/FATAL)")
   public void setRocksdbLogLevel(String level) {
     this.rocksdbLogLevel = level;
   }
@@ -59,14 +68,6 @@ public String getRocksdbLogLevel() {
     return rocksdbLogLevel;
   }
 
-  private boolean syncOption;
-  @Config(key = "rocksdb.writeoption.sync",
-      type = ConfigType.BOOLEAN,
-      defaultValue = "false",
-      tags = {ConfigTag.OM},
-      description = "Enable/Disable Sync option. If true write will be " +
-          "considered complete, once flushed to persistent storage. If false," +
-          " writes are flushed asynchronously.")
   public void setSyncOption(boolean enabled) {
     this.syncOption = enabled;
   }