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..1b0bcaf9abb 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 @@ -33,11 +33,11 @@ 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.PriorityQueue; import java.util.Set; import java.util.concurrent.locks.Lock; import java.util.concurrent.locks.ReadWriteLock; @@ -386,20 +386,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..7a40208e074 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 @@ -112,6 +112,7 @@ import java.util.List; import java.util.Map; import java.util.Map.Entry; +import java.util.PriorityQueue; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.locks.ReentrantReadWriteLock; @@ -1001,11 +1002,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)); - } } } }