From 487eaa6dea14bba100583f30f196c0d51215bd7c Mon Sep 17 00:00:00 2001
From: Ahmed Hussein <50450311+amahussein@users.noreply.github.com>
Date: Wed, 11 Nov 2020 14:39:03 -0600
Subject: [PATCH] HADOOP-17358. Improve excessive reloading of Configurations
 (#2436)

Co-authored-by: ahussein <ahmed.hussein@verizonmedia.com>
(cherry picked from commit 71071e5c0fcaf73a3989000dbc60fa214a317da1)
(cherry picked from commit 23fe3bdab3410b8ed370fa6af9f2ed63817006ab)
(cherry picked from commit ac3e10af63c4a70e45b26acd50b01a93fa8fd605)
---
 .../org/apache/hadoop/conf/Configuration.java | 34 ++++++++++++++-----
 .../conf/TestConfigurationSubclass.java       |  3 +-
 2 files changed, 27 insertions(+), 10 deletions(-)

diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java
index 5a708a882f6..8e7da62c4f7 100644
--- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java
+++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/conf/Configuration.java
@@ -1005,11 +1005,11 @@ public class Configuration implements Iterable<Map.Entry<String,String>>,
     properties = null;                            // trigger reload
     finalParameters.clear();                      // clear site-limits
   }
-  
+
   private synchronized void addResourceObject(Resource resource) {
     resources.add(resource);                      // add to resources
     restrictSystemProps |= resource.isParserRestricted();
-    reloadConfiguration();
+    loadProps(properties, resources.size() - 1, false);
   }
 
   private static final int MAX_SUBST = 20;
@@ -2843,12 +2843,27 @@ public class Configuration implements Iterable<Map.Entry<String,String>>,
   protected synchronized Properties getProps() {
     if (properties == null) {
       properties = new Properties();
-      Map<String, String[]> backup = updatingResource != null ?
-          new ConcurrentHashMap<String, String[]>(updatingResource) : null;
-      loadResources(properties, resources, quietmode);
+      loadProps(properties, 0, true);
+    }
+    return properties;
+  }
 
+  /**
+   * Loads the resource at a given index into the properties.
+   * @param props the object containing the loaded properties.
+   * @param startIdx the index where the new resource has been added.
+   * @param fullReload flag whether we do complete reload of the conf instead
+   *                   of just loading the new resource.
+   */
+  private synchronized void loadProps(final Properties props,
+      final int startIdx, final boolean fullReload) {
+    if (props != null) {
+      Map<String, String[]> backup =
+          updatingResource != null
+              ? new ConcurrentHashMap<>(updatingResource) : null;
+      loadResources(props, resources, startIdx, fullReload, quietmode);
       if (overlay != null) {
-        properties.putAll(overlay);
+        props.putAll(overlay);
         if (backup != null) {
           for (Map.Entry<Object, Object> item : overlay.entrySet()) {
             String key = (String) item.getKey();
@@ -2860,7 +2875,6 @@ public class Configuration implements Iterable<Map.Entry<String,String>>,
         }
       }
     }
-    return properties;
   }
 
   /**
@@ -2962,14 +2976,16 @@ public class Configuration implements Iterable<Map.Entry<String,String>>,
 
   private void loadResources(Properties properties,
                              ArrayList<Resource> resources,
+                             int startIdx,
+                             boolean fullReload,
                              boolean quiet) {
-    if(loadDefaults) {
+    if(loadDefaults && fullReload) {
       for (String resource : defaultResources) {
         loadResource(properties, new Resource(resource, false), quiet);
       }
     }
     
-    for (int i = 0; i < resources.size(); i++) {
+    for (int i = startIdx; i < resources.size(); i++) {
       Resource ret = loadResource(properties, resources.get(i), quiet);
       if (ret != null) {
         resources.set(i, ret);
diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/conf/TestConfigurationSubclass.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/conf/TestConfigurationSubclass.java
index e15e699534d..51d23d8038b 100644
--- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/conf/TestConfigurationSubclass.java
+++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/conf/TestConfigurationSubclass.java
@@ -53,8 +53,9 @@ public class TestConfigurationSubclass {
     SubConf conf = new SubConf(true);
     conf.setQuietMode(false);
     assertFalse(conf.isReloaded());
+    // adding a resource does not force a reload.
     conf.addResource("not-a-valid-resource");
-    assertTrue(conf.isReloaded());
+    assertFalse(conf.isReloaded());
     try {
       Properties properties = conf.getProperties();
       fail("Should not have got here");