Pārlūkot izejas kodu

HADOOP-19597. Log warning message on every set/get of a deprecated configuration property (#7766) Contributed by Stamatis Zampetakis.

* HADOOP-19597. Log warning message on every set/get of a deprecated configuration property

Reviewed-by: Shilun Fan <slfan1989@apache.org>
Signed-off-by: Shilun Fan <slfan1989@apache.org>
Stamatis Zampetakis 2 dienas atpakaļ
vecāks
revīzija
f93aff5b68

+ 11 - 25
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java

@@ -379,10 +379,6 @@ public class Configuration implements Iterable<Map.Entry<String,String>>,
       this.customMessage = customMessage;
     }
 
-    private final String getWarningMessage(String key) {
-      return getWarningMessage(key, null);
-    }
-
     /**
      * Method to provide the warning message. It gives the custom message if
      * non-null, and default message otherwise.
@@ -412,12 +408,9 @@ public class Configuration implements Iterable<Map.Entry<String,String>>,
       return warningMessage;
     }
 
-    boolean getAndSetAccessed() {
-      return accessed.getAndSet(true);
-    }
-
-    public void clearAccessed() {
-      accessed.set(false);
+    void logDeprecation(String name, String source) {
+      LOG_DEPRECATION.info(getWarningMessage(name, source));
+      this.accessed.set(true);
     }
   }
   
@@ -728,12 +721,10 @@ public class Configuration implements Iterable<Map.Entry<String,String>>,
     }
     // Initialize the return value with requested name
     String[] names = new String[]{name};
-    // Deprecated keys are logged once and an updated names are returned
+    // Deprecated keys are logged and updated names are returned
     DeprecatedKeyInfo keyInfo = deprecations.getDeprecatedKeyMap().get(name);
     if (keyInfo != null) {
-      if (!keyInfo.getAndSetAccessed()) {
-        logDeprecation(keyInfo.getWarningMessage(name));
-      }
+      keyInfo.logDeprecation(name, null);
       // Override return value for deprecated keys
       names = keyInfo.newKeys;
     }
@@ -1462,13 +1453,6 @@ public class Configuration implements Iterable<Map.Entry<String,String>>,
     LOG_DEPRECATION.info(message);
   }
 
-  void logDeprecationOnce(String name, String source) {
-    DeprecatedKeyInfo keyInfo = getDeprecatedKeyInfo(name);
-    if (keyInfo != null && !keyInfo.getAndSetAccessed()) {
-      LOG_DEPRECATION.info(keyInfo.getWarningMessage(name, source));
-    }
-  }
-
   /**
    * Unset a previously set property.
    * @param name the property name
@@ -2448,7 +2432,10 @@ public class Configuration implements Iterable<Map.Entry<String,String>>,
     if (oldName != null) {
       entry = provider.getCredentialEntry(oldName);
       if (entry != null) {
-        logDeprecationOnce(oldName, provider.toString());
+        DeprecatedKeyInfo ki = getDeprecatedKeyInfo(oldName);
+        if (ki != null) {
+          ki.logDeprecation(oldName, provider.toString());
+        }
         return entry;
       }
     }
@@ -2459,7 +2446,7 @@ public class Configuration implements Iterable<Map.Entry<String,String>>,
       for (String newName : keyInfo.newKeys) {
         entry = provider.getCredentialEntry(newName);
         if (entry != null) {
-          logDeprecationOnce(name, null);
+          keyInfo.logDeprecation(name, null);
           return entry;
         }
       }
@@ -3433,8 +3420,7 @@ public class Configuration implements Iterable<Map.Entry<String,String>>,
           deprecations.getDeprecatedKeyMap().get(confName);
 
       if (keyInfo != null) {
-        logDeprecation(keyInfo.getWarningMessage(confName, wrapper.toString()));
-        keyInfo.clearAccessed();
+        keyInfo.logDeprecation(confName, wrapper.toString());
         for (String key : keyInfo.newKeys) {
           // update new keys with deprecated key's value
           results.add(new ParsedItem(

+ 30 - 0
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/conf/TestConfiguration.java

@@ -384,6 +384,36 @@ public class TestConfiguration {
     assertTrue(hasDeprecationMessage);
   }
 
+  @Test
+  public void testDeprecatedPropertyLogsWarningOnEveryUse(){
+    String oldProp = "test.deprecation.old.conf.b";
+    String newProp = "test.deprecation.new.conf.b";
+    Configuration.addDeprecation(oldProp, newProp);
+
+    TestAppender appender = new TestAppender();
+    Logger deprecationLogger = Logger.getLogger("org.apache.hadoop.conf.Configuration.deprecation");
+    deprecationLogger.addAppender(appender);
+
+    try {
+      conf.set(oldProp, "b1");
+      conf.get(oldProp);
+      conf.set(oldProp, "b2");
+      conf.get(oldProp);
+      // Using the new property should not log a warning
+      conf.set(newProp, "b3");
+      conf.get(newProp);
+      conf.set(newProp, "b4");
+      conf.get(newProp);
+    } finally {
+      deprecationLogger.removeAppender(appender);
+    }
+
+    Pattern deprecationMsgPattern = Pattern.compile(oldProp + " is deprecated");
+    long count = appender.log.stream().map(LoggingEvent::getRenderedMessage)
+            .filter(msg -> deprecationMsgPattern.matcher(msg).find()).count();
+    assertEquals(4, count, "Expected exactly four warnings for deprecated property usage");
+  }
+
   /**
    * A simple appender for white box testing.
    */