diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/ClusterNodeTracker.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/ClusterNodeTracker.java index f4491df7bf1..8f0ef9d034c 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/ClusterNodeTracker.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/ClusterNodeTracker.java @@ -31,14 +31,7 @@ import org.apache.hadoop.yarn.util.resource.Resources; import org.apache.hadoop.yarn.util.resource.ResourceUtils; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collections; -import java.util.Comparator; -import java.util.HashMap; -import java.util.List; -import java.util.Map; -import java.util.Set; +import java.util.*; import java.util.concurrent.locks.Lock; import java.util.concurrent.locks.ReadWriteLock; import java.util.concurrent.locks.ReentrantReadWriteLock; @@ -386,20 +379,21 @@ private void updateMaxResources(SchedulerNode node, boolean add) { /** * Convenience method to sort nodes. + * Nodes can change while being sorted. Using a standard sort will fail + * without locking each node, the PriorityQueue handles this without locks. * - * Note that the sort is performed without holding a lock. We are sorting - * here instead of on the caller to allow for future optimizations (e.g. - * sort once every x milliseconds). + * @param comparator the comparator to sort the nodes with + * @return sorted list in the form of a PriorityQueue */ - public List sortedNodeList(Comparator comparator) { - List sortedList = null; + public PriorityQueue sortedNodeList(Comparator comparator) { + PriorityQueue sortedList = + new PriorityQueue<>(nodes.size() < 1 ? 1 : nodes.size(), comparator); readLock.lock(); try { - sortedList = new ArrayList(nodes.values()); + sortedList.addAll(nodes.values()); } finally { readLock.unlock(); } - Collections.sort(sortedList, comparator); return sortedList; } diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/FairScheduler.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/FairScheduler.java index 04bbe0ff684..971735c8eaf 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/FairScheduler.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/FairScheduler.java @@ -103,16 +103,8 @@ import org.apache.hadoop.yarn.util.resource.Resources; import java.io.IOException; -import java.util.ArrayList; -import java.util.Collection; -import java.util.Collections; -import java.util.Comparator; -import java.util.EnumSet; -import java.util.HashSet; -import java.util.List; -import java.util.Map; +import java.util.*; import java.util.Map.Entry; -import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.locks.ReentrantReadWriteLock; import java.util.concurrent.locks.ReentrantReadWriteLock.ReadLock; @@ -1001,11 +993,13 @@ protected void nodeUpdate(RMNode nm) { @Deprecated void continuousSchedulingAttempt() throws InterruptedException { long start = getClock().getTime(); - List nodeIdList; - // Hold a lock to prevent comparator order changes due to changes of node - // unallocated resources - synchronized (this) { + PriorityQueue nodeIdList; + // Hold a lock to prevent node changes as much as possible. + readLock.lock(); + try { nodeIdList = nodeTracker.sortedNodeList(nodeAvailableResourceComparator); + } finally { + readLock.unlock(); } // iterate all nodes diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/TestContinuousScheduling.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/TestContinuousScheduling.java index 5dac8622164..895962d6094 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/TestContinuousScheduling.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/fair/TestContinuousScheduling.java @@ -335,9 +335,7 @@ public void run() { for (int j = 0; j < 100; j++) { for (FSSchedulerNode node : clusterNodeTracker.getAllNodes()) { int i = ThreadLocalRandom.current().nextInt(-30, 30); - synchronized (scheduler) { node.deductUnallocatedResource(Resource.newInstance(i * 1024, i)); - } } } }