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 <[email protected]> Authored: Wed May 17 08:26:29 2017 +0530 Committer: Rohith Sharma K S <[email protected]> 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<String> includes; - // exclude host list with optional timeout. - // If the value is null, it indicates default timeout. - private Map<String, Integer> 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<HostDetails> current; + + public HostsFileReader(String inFile, String exFile) throws IOException { - includes = new HashSet<String>(); - excludes = new HashMap<String, Integer>(); - 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<String>(); - excludes = new HashMap<String, Integer>(); - 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<String> newIncludes = new HashSet<String>(); - Map<String, Integer> newExcludes = new HashMap<String, Integer>(); - 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<String> newIncludes = oldDetails.includes; + Map<String, Integer> 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<String> newIncludes = new HashSet<String>(); - Map<String, Integer> newExcludes = new HashMap<String, Integer>(); - 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<String> newIncludes = oldDetails.includes; + Map<String, Integer> 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<String> getHosts() { - this.readLock.lock(); - try { - return includes; - } finally { - this.readLock.unlock(); - } + HostDetails hostDetails = current.get(); + return hostDetails.getIncludedHosts(); } public Set<String> 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<String> includes, Set<String> 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<String> includeHosts, Map<String, Integer> 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<String> includes; + private final String excludesFile; + // exclude host list with optional timeout. + // If the value is null, it indicates default timeout. + private final Map<String, Integer> excludes; + + HostDetails(String includesFile, Set<String> includes, + String excludesFile, Map<String, Integer> excludes) { + this.includesFile = includesFile; + this.includes = includes; + this.excludesFile = excludesFile; + this.excludes = excludes; + } + + public String getIncludesFile() { + return includesFile; + } + + public Set<String> getIncludedHosts() { + return includes; + } + + public String getExcludesFile() { + return excludesFile; + } + + public Set<String> getExcludedHosts() { + return excludes.keySet(); + } + + public Map<String, Integer> 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<String> hostsList = new HashSet<String>(); - Set<String> excludeList = new HashSet<String>(); - 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<String> includes = new HashSet<String>(); - Map<String, Integer> excludes = new HashMap<String, Integer>(); - hfp.getHostDetails(includes, excludes); + HostDetails hostDetails = hfp.getHostDetails(); + Map<String, Integer> 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<String> hostsList = new HashSet<String>(); - Set<String> excludeList = new HashSet<String>(); - 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<RMNode> nodesToDecom = new ArrayList<RMNode>(); - Set<String> includes = new HashSet<String>(); - Map<String, Integer> excludes = new HashMap<String, Integer>(); - hostsReader.getHostDetails(includes, excludes); + HostDetails hostDetails = hostsReader.getHostDetails(); + Set<String> includes = hostDetails.getIncludedHosts(); + Map<String, Integer> 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<String> hostsList = new HashSet<String>(); - Set<String> excludeList = new HashSet<String>(); - 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<String> hostsList = new HashSet<String>(); - Set<String> excludeList = new HashSet<String>(); - hostsReader.getHostDetails(hostsList, excludeList); + HostDetails hostDetails = hostsReader.getHostDetails(); + Set<String> hostsList = hostDetails.getIncludedHosts(); + Set<String> excludeList = hostDetails.getExcludedHosts(); return !hostsList.isEmpty() && !hostsList.contains(hostName) && !hostsList.contains(ip) && !excludeList.contains(hostName) --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
