From d6e55262424634531a5256e717e9b1e1d44ad2f8 Mon Sep 17 00:00:00 2001 From: ahussein Date: Wed, 4 Nov 2020 15:25:52 -0600 Subject: [PATCH 1/3] HADOOP-17358. Improve excessive reloading of Configurations --- .../org/apache/hadoop/conf/Configuration.java | 36 ++++++++++++++----- .../conf/TestConfigurationSubclass.java | 3 +- 2 files changed, 29 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 db8699e44be42..3c4d41797fc82 100755 --- 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 @@ -1027,11 +1027,11 @@ public synchronized void reloadConfiguration() { 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; @@ -2876,12 +2876,28 @@ public Set getFinalParameters() { protected synchronized Properties getProps() { if (properties == null) { properties = new Properties(); - Map backup = updatingResource != null ? - new ConcurrentHashMap(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. + * @return the properties loaded from the resource. + */ + private synchronized Properties loadProps(final Properties props, + final int startIdx, final boolean fullReload) { + if (props != null) { + Map 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 item : overlay.entrySet()) { String key = (String) item.getKey(); @@ -2893,7 +2909,7 @@ protected synchronized Properties getProps() { } } } - return properties; + return props; } /** @@ -2995,14 +3011,16 @@ private XMLStreamReader parse(InputStream is, String systemIdStr, private void loadResources(Properties properties, ArrayList 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 e15e699534d31..51d23d8038b0b 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 void testReloadNotQuiet() throws Throwable { 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"); From d3087c2c1956f5eba6dc2cca26e4cfa93fa4b2d2 Mon Sep 17 00:00:00 2001 From: ahussein Date: Tue, 10 Nov 2020 14:57:38 -0600 Subject: [PATCH 2/3] HADOOP-17358. address reviews --- .../src/main/java/org/apache/hadoop/conf/Configuration.java | 3 +-- 1 file changed, 1 insertion(+), 2 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 3c4d41797fc82..4d4c58d188e16 100755 --- 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 @@ -2889,7 +2889,7 @@ protected synchronized Properties getProps() { * of just loading the new resource. * @return the properties loaded from the resource. */ - private synchronized Properties loadProps(final Properties props, + private synchronized void loadProps(final Properties props, final int startIdx, final boolean fullReload) { if (props != null) { Map backup = @@ -2909,7 +2909,6 @@ private synchronized Properties loadProps(final Properties props, } } } - return props; } /** From 3b5256509a0894779d2b7b09f86aa64431538900 Mon Sep 17 00:00:00 2001 From: ahussein Date: Tue, 10 Nov 2020 17:54:55 -0600 Subject: [PATCH 3/3] HADOOP-1353. remove return from javadoc --- .../src/main/java/org/apache/hadoop/conf/Configuration.java | 1 - 1 file changed, 1 deletion(-) 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 4d4c58d188e16..e0e7ac3960451 100755 --- 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 @@ -2887,7 +2887,6 @@ protected synchronized Properties getProps() { * @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. - * @return the properties loaded from the resource. */ private synchronized void loadProps(final Properties props, final int startIdx, final boolean fullReload) {