Return-Path: X-Original-To: archive-asf-public-internal@cust-asf2.ponee.io Delivered-To: archive-asf-public-internal@cust-asf2.ponee.io Received: from cust-asf.ponee.io (cust-asf.ponee.io [163.172.22.183]) by cust-asf2.ponee.io (Postfix) with ESMTP id 5B031200C8C for ; Mon, 22 May 2017 22:48:12 +0200 (CEST) Received: by cust-asf.ponee.io (Postfix) id 59BB1160BD8; Mon, 22 May 2017 20:48:12 +0000 (UTC) Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by cust-asf.ponee.io (Postfix) with SMTP id 2C197160BBF for ; Mon, 22 May 2017 22:48:11 +0200 (CEST) Received: (qmail 44679 invoked by uid 500); 22 May 2017 20:48:07 -0000 Mailing-List: contact common-commits-help@hadoop.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Delivered-To: mailing list common-commits@hadoop.apache.org Received: (qmail 41858 invoked by uid 99); 22 May 2017 20:48:05 -0000 Received: from git1-us-west.apache.org (HELO git1-us-west.apache.org) (140.211.11.23) by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 22 May 2017 20:48:05 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id DDB7EE1863; Mon, 22 May 2017 20:48:05 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: wangda@apache.org To: common-commits@hadoop.apache.org Date: Mon, 22 May 2017 20:48:29 -0000 Message-Id: <16ce5e6bbae34daa8fa6b2e1167ba226@git.apache.org> In-Reply-To: <479084d7b60c42579b2d200c2d117203@git.apache.org> References: <479084d7b60c42579b2d200c2d117203@git.apache.org> X-Mailer: ASF-Git Admin Mailer Subject: [25/50] hadoop git commit: HADOOP-14412. HostsFileReader#getHostDetails is very expensive on large clusters. Contributed by Jason Lowe. archived-at: Mon, 22 May 2017 20:48:12 -0000 HADOOP-14412. HostsFileReader#getHostDetails is very expensive on large clusters. Contributed by Jason Lowe. Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/d87a63a9 Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/d87a63a9 Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/d87a63a9 Branch: refs/heads/YARN-5734 Commit: d87a63a9019d74a1c338c724e050952843a153e5 Parents: ec21ce4 Author: Rohith Sharma K S Authored: Wed May 17 08:26:29 2017 +0530 Committer: Rohith Sharma K S Committed: Wed May 17 08:27:45 2017 +0530 ---------------------------------------------------------------------- .../org/apache/hadoop/util/HostsFileReader.java | 263 ++++++++++--------- .../apache/hadoop/util/TestHostsFileReader.java | 19 +- .../resourcemanager/NodesListManager.java | 30 +-- 3 files changed, 161 insertions(+), 151 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hadoop/blob/d87a63a9/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/HostsFileReader.java ---------------------------------------------------------------------- diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/HostsFileReader.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/HostsFileReader.java index 2ef1ead..2913b87 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/HostsFileReader.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/HostsFileReader.java @@ -20,13 +20,12 @@ package org.apache.hadoop.util; import java.io.*; import java.nio.charset.StandardCharsets; +import java.util.Collections; import java.util.Set; import java.util.HashMap; import java.util.HashSet; -import java.util.concurrent.locks.ReentrantReadWriteLock; -import java.util.concurrent.locks.ReentrantReadWriteLock.ReadLock; -import java.util.concurrent.locks.ReentrantReadWriteLock.WriteLock; import java.util.Map; +import java.util.concurrent.atomic.AtomicReference; import javax.xml.parsers.DocumentBuilder; import javax.xml.parsers.DocumentBuilderFactory; @@ -48,39 +47,26 @@ import org.xml.sax.SAXException; @InterfaceAudience.LimitedPrivate({"HDFS", "MapReduce"}) @InterfaceStability.Unstable public class HostsFileReader { - private Set includes; - // exclude host list with optional timeout. - // If the value is null, it indicates default timeout. - private Map excludes; - private String includesFile; - private String excludesFile; - private WriteLock writeLock; - private ReadLock readLock; - private static final Log LOG = LogFactory.getLog(HostsFileReader.class); - public HostsFileReader(String inFile, + private final AtomicReference current; + + public HostsFileReader(String inFile, String exFile) throws IOException { - includes = new HashSet(); - excludes = new HashMap(); - includesFile = inFile; - excludesFile = exFile; - ReentrantReadWriteLock rwLock = new ReentrantReadWriteLock(); - this.writeLock = rwLock.writeLock(); - this.readLock = rwLock.readLock(); - refresh(); + HostDetails hostDetails = new HostDetails( + inFile, Collections.emptySet(), + exFile, Collections.emptyMap()); + current = new AtomicReference<>(hostDetails); + refresh(inFile, exFile); } @Private public HostsFileReader(String includesFile, InputStream inFileInputStream, String excludesFile, InputStream exFileInputStream) throws IOException { - includes = new HashSet(); - excludes = new HashMap(); - this.includesFile = includesFile; - this.excludesFile = excludesFile; - ReentrantReadWriteLock rwLock = new ReentrantReadWriteLock(); - this.writeLock = rwLock.writeLock(); - this.readLock = rwLock.readLock(); + HostDetails hostDetails = new HostDetails( + includesFile, Collections.emptySet(), + excludesFile, Collections.emptyMap()); + current = new AtomicReference<>(hostDetails); refresh(inFileInputStream, exFileInputStream); } @@ -126,12 +112,8 @@ public class HostsFileReader { } public void refresh() throws IOException { - this.writeLock.lock(); - try { - refresh(includesFile, excludesFile); - } finally { - this.writeLock.unlock(); - } + HostDetails hostDetails = current.get(); + refresh(hostDetails.includesFile, hostDetails.excludesFile); } public static void readFileToMap(String type, @@ -201,128 +183,163 @@ public class HostsFileReader { return (nodes.getLength() == 0)? null : nodes.item(0).getTextContent(); } - public void refresh(String includeFiles, String excludeFiles) + public void refresh(String includesFile, String excludesFile) throws IOException { LOG.info("Refreshing hosts (include/exclude) list"); - this.writeLock.lock(); - try { - // update instance variables - updateFileNames(includeFiles, excludeFiles); - Set newIncludes = new HashSet(); - Map newExcludes = new HashMap(); - boolean switchIncludes = false; - boolean switchExcludes = false; - if (includeFiles != null && !includeFiles.isEmpty()) { - readFileToSet("included", includeFiles, newIncludes); - switchIncludes = true; - } - if (excludeFiles != null && !excludeFiles.isEmpty()) { - readFileToMap("excluded", excludeFiles, newExcludes); - switchExcludes = true; - } - - if (switchIncludes) { - // switch the new hosts that are to be included - includes = newIncludes; - } - if (switchExcludes) { - // switch the excluded hosts - excludes = newExcludes; - } - } finally { - this.writeLock.unlock(); + HostDetails oldDetails = current.get(); + Set newIncludes = oldDetails.includes; + Map newExcludes = oldDetails.excludes; + if (includesFile != null && !includesFile.isEmpty()) { + newIncludes = new HashSet<>(); + readFileToSet("included", includesFile, newIncludes); + newIncludes = Collections.unmodifiableSet(newIncludes); + } + if (excludesFile != null && !excludesFile.isEmpty()) { + newExcludes = new HashMap<>(); + readFileToMap("excluded", excludesFile, newExcludes); + newExcludes = Collections.unmodifiableMap(newExcludes); } + HostDetails newDetails = new HostDetails(includesFile, newIncludes, + excludesFile, newExcludes); + current.set(newDetails); } @Private public void refresh(InputStream inFileInputStream, InputStream exFileInputStream) throws IOException { LOG.info("Refreshing hosts (include/exclude) list"); - this.writeLock.lock(); - try { - Set newIncludes = new HashSet(); - Map newExcludes = new HashMap(); - boolean switchIncludes = false; - boolean switchExcludes = false; - if (inFileInputStream != null) { - readFileToSetWithFileInputStream("included", includesFile, - inFileInputStream, newIncludes); - switchIncludes = true; - } - if (exFileInputStream != null) { - readFileToMapWithFileInputStream("excluded", excludesFile, - exFileInputStream, newExcludes); - switchExcludes = true; - } - if (switchIncludes) { - // switch the new hosts that are to be included - includes = newIncludes; - } - if (switchExcludes) { - // switch the excluded hosts - excludes = newExcludes; - } - } finally { - this.writeLock.unlock(); + HostDetails oldDetails = current.get(); + Set newIncludes = oldDetails.includes; + Map newExcludes = oldDetails.excludes; + if (inFileInputStream != null) { + newIncludes = new HashSet<>(); + readFileToSetWithFileInputStream("included", oldDetails.includesFile, + inFileInputStream, newIncludes); + newIncludes = Collections.unmodifiableSet(newIncludes); + } + if (exFileInputStream != null) { + newExcludes = new HashMap<>(); + readFileToMapWithFileInputStream("excluded", oldDetails.excludesFile, + exFileInputStream, newExcludes); + newExcludes = Collections.unmodifiableMap(newExcludes); } + HostDetails newDetails = new HostDetails( + oldDetails.includesFile, newIncludes, + oldDetails.excludesFile, newExcludes); + current.set(newDetails); } public Set getHosts() { - this.readLock.lock(); - try { - return includes; - } finally { - this.readLock.unlock(); - } + HostDetails hostDetails = current.get(); + return hostDetails.getIncludedHosts(); } public Set getExcludedHosts() { - this.readLock.lock(); - try { - return excludes.keySet(); - } finally { - this.readLock.unlock(); - } + HostDetails hostDetails = current.get(); + return hostDetails.getExcludedHosts(); } + /** + * Retrieve an atomic view of the included and excluded hosts. + * + * @param includes set to populate with included hosts + * @param excludes set to populate with excluded hosts + * @deprecated use {@link #getHostDetails() instead} + */ + @Deprecated public void getHostDetails(Set includes, Set excludes) { - this.readLock.lock(); - try { - includes.addAll(this.includes); - excludes.addAll(this.excludes.keySet()); - } finally { - this.readLock.unlock(); - } + HostDetails hostDetails = current.get(); + includes.addAll(hostDetails.getIncludedHosts()); + excludes.addAll(hostDetails.getExcludedHosts()); } + /** + * Retrieve an atomic view of the included and excluded hosts. + * + * @param includeHosts set to populate with included hosts + * @param excludeHosts map to populate with excluded hosts + * @deprecated use {@link #getHostDetails() instead} + */ + @Deprecated public void getHostDetails(Set includeHosts, Map excludeHosts) { - this.readLock.lock(); - try { - includeHosts.addAll(this.includes); - excludeHosts.putAll(this.excludes); - } finally { - this.readLock.unlock(); - } + HostDetails hostDetails = current.get(); + includeHosts.addAll(hostDetails.getIncludedHosts()); + excludeHosts.putAll(hostDetails.getExcludedMap()); + } + + /** + * Retrieve an atomic view of the included and excluded hosts. + * + * @return the included and excluded hosts + */ + public HostDetails getHostDetails() { + return current.get(); } public void setIncludesFile(String includesFile) { LOG.info("Setting the includes file to " + includesFile); - this.includesFile = includesFile; + HostDetails oldDetails = current.get(); + HostDetails newDetails = new HostDetails(includesFile, oldDetails.includes, + oldDetails.excludesFile, oldDetails.excludes); + current.set(newDetails); } public void setExcludesFile(String excludesFile) { LOG.info("Setting the excludes file to " + excludesFile); - this.excludesFile = excludesFile; + HostDetails oldDetails = current.get(); + HostDetails newDetails = new HostDetails( + oldDetails.includesFile, oldDetails.includes, + excludesFile, oldDetails.excludes); + current.set(newDetails); } - public void updateFileNames(String includeFiles, String excludeFiles) { - this.writeLock.lock(); - try { - setIncludesFile(includeFiles); - setExcludesFile(excludeFiles); - } finally { - this.writeLock.unlock(); + public void updateFileNames(String includesFile, String excludesFile) { + LOG.info("Setting the includes file to " + includesFile); + LOG.info("Setting the excludes file to " + excludesFile); + HostDetails oldDetails = current.get(); + HostDetails newDetails = new HostDetails(includesFile, oldDetails.includes, + excludesFile, oldDetails.excludes); + current.set(newDetails); + } + + /** + * An atomic view of the included and excluded hosts + */ + public static class HostDetails { + private final String includesFile; + private final Set includes; + private final String excludesFile; + // exclude host list with optional timeout. + // If the value is null, it indicates default timeout. + private final Map excludes; + + HostDetails(String includesFile, Set includes, + String excludesFile, Map excludes) { + this.includesFile = includesFile; + this.includes = includes; + this.excludesFile = excludesFile; + this.excludes = excludes; + } + + public String getIncludesFile() { + return includesFile; + } + + public Set getIncludedHosts() { + return includes; + } + + public String getExcludesFile() { + return excludesFile; + } + + public Set getExcludedHosts() { + return excludes.keySet(); + } + + public Map getExcludedMap() { + return excludes; } } } http://git-wip-us.apache.org/repos/asf/hadoop/blob/d87a63a9/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestHostsFileReader.java ---------------------------------------------------------------------- diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestHostsFileReader.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestHostsFileReader.java index 5766591..2462114 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestHostsFileReader.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestHostsFileReader.java @@ -20,12 +20,10 @@ package org.apache.hadoop.util; import java.io.File; import java.io.FileNotFoundException; import java.io.FileWriter; -import java.util.HashMap; -import java.util.HashSet; -import java.util.Set; import java.util.Map; import org.apache.hadoop.test.GenericTestUtils; +import org.apache.hadoop.util.HostsFileReader.HostDetails; import org.junit.*; import static org.junit.Assert.*; @@ -121,11 +119,11 @@ public class TestHostsFileReader { assertTrue(hfp.getExcludedHosts().contains("node1")); assertTrue(hfp.getHosts().contains("node2")); - Set hostsList = new HashSet(); - Set excludeList = new HashSet(); - hfp.getHostDetails(hostsList, excludeList); - assertTrue(excludeList.contains("node1")); - assertTrue(hostsList.contains("node2")); + HostDetails hostDetails = hfp.getHostDetails(); + assertTrue(hostDetails.getExcludedHosts().contains("node1")); + assertTrue(hostDetails.getIncludedHosts().contains("node2")); + assertEquals(newIncludesFile, hostDetails.getIncludesFile()); + assertEquals(newExcludesFile, hostDetails.getExcludesFile()); } /* @@ -328,9 +326,8 @@ public class TestHostsFileReader { assertEquals(4, includesLen); assertEquals(9, excludesLen); - Set includes = new HashSet(); - Map excludes = new HashMap(); - hfp.getHostDetails(includes, excludes); + HostDetails hostDetails = hfp.getHostDetails(); + Map excludes = hostDetails.getExcludedMap(); assertTrue(excludes.containsKey("host1")); assertTrue(excludes.containsKey("host2")); assertTrue(excludes.containsKey("host3")); http://git-wip-us.apache.org/repos/asf/hadoop/blob/d87a63a9/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/NodesListManager.java ---------------------------------------------------------------------- diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/NodesListManager.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/NodesListManager.java index 7d69f93..edd173b 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/NodesListManager.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/NodesListManager.java @@ -20,7 +20,6 @@ package org.apache.hadoop.yarn.server.resourcemanager; import java.io.IOException; import java.util.ArrayList; -import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; import java.util.List; @@ -40,6 +39,7 @@ import org.apache.hadoop.net.Node; import org.apache.hadoop.service.AbstractService; import org.apache.hadoop.service.CompositeService; import org.apache.hadoop.util.HostsFileReader; +import org.apache.hadoop.util.HostsFileReader.HostDetails; import org.apache.hadoop.util.Time; import org.apache.hadoop.util.StringUtils; import org.apache.hadoop.yarn.api.records.NodeId; @@ -192,14 +192,11 @@ public class NodesListManager extends CompositeService implements conf.get(YarnConfiguration.RM_NODES_EXCLUDE_FILE_PATH, YarnConfiguration.DEFAULT_RM_NODES_EXCLUDE_FILE_PATH)); - Set hostsList = new HashSet(); - Set excludeList = new HashSet(); - hostsReader.getHostDetails(hostsList, excludeList); - - for (String include : hostsList) { + HostDetails hostDetails = hostsReader.getHostDetails(); + for (String include : hostDetails.getIncludedHosts()) { LOG.debug("include: " + include); } - for (String exclude : excludeList) { + for (String exclude : hostDetails.getExcludedHosts()) { LOG.debug("exclude: " + exclude); } } @@ -262,9 +259,9 @@ public class NodesListManager extends CompositeService implements // Nodes need to be decommissioned (graceful or forceful); List nodesToDecom = new ArrayList(); - Set includes = new HashSet(); - Map excludes = new HashMap(); - hostsReader.getHostDetails(includes, excludes); + HostDetails hostDetails = hostsReader.getHostDetails(); + Set includes = hostDetails.getIncludedHosts(); + Map excludes = hostDetails.getExcludedMap(); for (RMNode n : this.rmContext.getRMNodes().values()) { NodeState s = n.getState(); @@ -453,10 +450,9 @@ public class NodesListManager extends CompositeService implements } public boolean isValidNode(String hostName) { - Set hostsList = new HashSet(); - Set excludeList = new HashSet(); - hostsReader.getHostDetails(hostsList, excludeList); - return isValidNode(hostName, hostsList, excludeList); + HostDetails hostDetails = hostsReader.getHostDetails(); + return isValidNode(hostName, hostDetails.getIncludedHosts(), + hostDetails.getExcludedHosts()); } private boolean isValidNode( @@ -563,9 +559,9 @@ public class NodesListManager extends CompositeService implements public boolean isUntrackedNode(String hostName) { String ip = resolver.resolve(hostName); - Set hostsList = new HashSet(); - Set excludeList = new HashSet(); - hostsReader.getHostDetails(hostsList, excludeList); + HostDetails hostDetails = hostsReader.getHostDetails(); + Set hostsList = hostDetails.getIncludedHosts(); + Set excludeList = hostDetails.getExcludedHosts(); return !hostsList.isEmpty() && !hostsList.contains(hostName) && !hostsList.contains(ip) && !excludeList.contains(hostName) --------------------------------------------------------------------- To unsubscribe, e-mail: common-commits-unsubscribe@hadoop.apache.org For additional commands, e-mail: common-commits-help@hadoop.apache.org