From 30fc5801966feb7f9bdd7d79db75acc595102913 Mon Sep 17 00:00:00 2001 From: Naganarasimha Date: Mon, 1 May 2017 20:15:27 +0530 Subject: [PATCH] YARN-6519. Fix warnings from Spotbugs in hadoop-yarn-server-resourcemanager. Contributed by Weiwei Yang. --- .../ApplicationMasterService.java | 2 +- .../ProportionalCapacityPreemptionPolicy.java | 17 ++++++--------- .../rmapp/attempt/RMAppAttemptImpl.java | 6 ++++-- .../rmapp/attempt/RMAppAttemptMetrics.java | 2 +- .../scheduler/AbstractYarnScheduler.java | 2 +- .../resourcemanager/scheduler/NodeType.java | 12 +++++++++-- .../scheduler/QueueMetrics.java | 21 +++++++++++++------ .../scheduler/capacity/CSQueueMetrics.java | 4 ++-- .../CapacitySchedulerQueueManager.java | 6 ++++-- .../scheduler/fair/FSQueueMetrics.java | 4 ++-- .../scheduler/fair/FSSchedulerNode.java | 8 +++---- 11 files changed, 50 insertions(+), 34 deletions(-) diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/ApplicationMasterService.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/ApplicationMasterService.java index 70a46a10a9..55b8fbbc46 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/ApplicationMasterService.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/ApplicationMasterService.java @@ -393,7 +393,7 @@ public boolean hasApplicationMasterRegistered( return hasApplicationMasterRegistered; } - protected final static List EMPTY_CONTAINER_LIST = + private final static List EMPTY_CONTAINER_LIST = new ArrayList(); protected static final Allocation EMPTY_ALLOCATION = new Allocation( EMPTY_CONTAINER_LIST, Resources.createResource(0), null, null, null); diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/monitor/capacity/ProportionalCapacityPreemptionPolicy.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/monitor/capacity/ProportionalCapacityPreemptionPolicy.java index 3bf69948d5..dc6f1c205d 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/monitor/capacity/ProportionalCapacityPreemptionPolicy.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/monitor/capacity/ProportionalCapacityPreemptionPolicy.java @@ -52,7 +52,6 @@ import java.util.Collections; import java.util.HashMap; import java.util.HashSet; -import java.util.Iterator; import java.util.LinkedHashSet; import java.util.List; import java.util.Map; @@ -306,16 +305,12 @@ private void syncKillableContainersFromScheduler() { private void cleanupStaledPreemptionCandidates(long currentTime) { // Keep the preemptionCandidates list clean - for (Iterator i = preemptionCandidates.keySet().iterator(); - i.hasNext(); ) { - RMContainer id = i.next(); - // garbage collect containers that are irrelevant for preemption - // And avoid preempt selected containers for *this execution* - // or within 1 ms - if (preemptionCandidates.get(id) + 2 * maxWaitTime < currentTime) { - i.remove(); - } - } + // garbage collect containers that are irrelevant for preemption + // And avoid preempt selected containers for *this execution* + // or within 1 ms + preemptionCandidates.entrySet() + .removeIf(candidate -> + candidate.getValue() + 2 * maxWaitTime < currentTime); } private Set getLeafQueueNames(TempQueuePerPartition q) { diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/rmapp/attempt/RMAppAttemptImpl.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/rmapp/attempt/RMAppAttemptImpl.java index 19503e519f..d66a97d263 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/rmapp/attempt/RMAppAttemptImpl.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/rmapp/attempt/RMAppAttemptImpl.java @@ -1000,9 +1000,11 @@ public void transferStateFromAttempt(RMAppAttempt attempt) { // if am crashed and not received this response, we should resend // this msg again after am restart if (!this.finishedContainersSentToAM.isEmpty()) { - for (NodeId nodeId : this.finishedContainersSentToAM.keySet()) { + for (Map.Entry> finishedContainer + : this.finishedContainersSentToAM.entrySet()) { List containerStatuses = - this.finishedContainersSentToAM.get(nodeId); + finishedContainer.getValue(); + NodeId nodeId = finishedContainer.getKey(); this.justFinishedContainers.putIfAbsent(nodeId, new ArrayList<>()); this.justFinishedContainers.get(nodeId).addAll(containerStatuses); } diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/rmapp/attempt/RMAppAttemptMetrics.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/rmapp/attempt/RMAppAttemptMetrics.java index a642e450fd..e089050de6 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/rmapp/attempt/RMAppAttemptMetrics.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/rmapp/attempt/RMAppAttemptMetrics.java @@ -152,7 +152,7 @@ public void updateAggregatePreemptedAppResourceUsage( public void incNumAllocatedContainers(NodeType containerType, NodeType requestType) { - localityStatistics[containerType.index][requestType.index]++; + localityStatistics[containerType.getIndex()][requestType.getIndex()]++; totalAllocatedContainers++; } 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/AbstractYarnScheduler.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/AbstractYarnScheduler.java index b954bdf029..c00b7bed3d 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/AbstractYarnScheduler.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/AbstractYarnScheduler.java @@ -132,7 +132,7 @@ public abstract class AbstractYarnScheduler protected int nmExpireInterval; protected long nmHeartbeatInterval; - protected final static List EMPTY_CONTAINER_LIST = + private final static List EMPTY_CONTAINER_LIST = new ArrayList(); protected static final Allocation EMPTY_ALLOCATION = new Allocation( EMPTY_CONTAINER_LIST, Resources.createResource(0), null, null, null); 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/NodeType.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/NodeType.java index 2b193bbb86..7bd15f0cfa 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/NodeType.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/NodeType.java @@ -23,9 +23,17 @@ */ public enum NodeType { NODE_LOCAL(0), RACK_LOCAL(1), OFF_SWITCH(2); - public int index; - private NodeType(int index) { + private final int index; + + NodeType(int index) { this.index = index; } + + /** + * @return the index of the node type + */ + public int getIndex() { + return index; + } } 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/QueueMetrics.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/QueueMetrics.java index 007d2b3b9f..9a57876d54 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/QueueMetrics.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/QueueMetrics.java @@ -142,20 +142,29 @@ static QueueMetrics forQueue(String queueName, Queue parent, */ @Private public synchronized static void clearQueueMetrics() { - queueMetrics.clear(); + QUEUE_METRICS.clear(); } - + /** * Simple metrics cache to help prevent re-registrations. */ - protected final static Map queueMetrics = + private static final Map QUEUE_METRICS = new HashMap(); - + + /** + * Returns the metrics cache to help prevent re-registrations. + * + * @return A string to {@link QueueMetrics} map. + */ + protected static Map getQueueMetrics() { + return QUEUE_METRICS; + } + public synchronized static QueueMetrics forQueue(MetricsSystem ms, String queueName, Queue parent, boolean enableUserMetrics, Configuration conf) { - QueueMetrics metrics = queueMetrics.get(queueName); + QueueMetrics metrics = QUEUE_METRICS.get(queueName); if (metrics == null) { metrics = new QueueMetrics(ms, queueName, parent, enableUserMetrics, conf). @@ -168,7 +177,7 @@ static QueueMetrics forQueue(MetricsSystem ms, String queueName, sourceName(queueName).toString(), "Metrics for queue: " + queueName, metrics); } - queueMetrics.put(queueName, metrics); + QUEUE_METRICS.put(queueName, metrics); } return metrics; 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/capacity/CSQueueMetrics.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/CSQueueMetrics.java index a601b7b805..c4d19340c1 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/CSQueueMetrics.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/CSQueueMetrics.java @@ -115,7 +115,7 @@ public void setAbsoluteUsedCapacity(Float absoluteUsedCapacity) { public synchronized static CSQueueMetrics forQueue(String queueName, Queue parent, boolean enableUserMetrics, Configuration conf) { MetricsSystem ms = DefaultMetricsSystem.instance(); - QueueMetrics metrics = queueMetrics.get(queueName); + QueueMetrics metrics = QueueMetrics.getQueueMetrics().get(queueName); if (metrics == null) { metrics = new CSQueueMetrics(ms, queueName, parent, enableUserMetrics, conf) @@ -127,7 +127,7 @@ public synchronized static CSQueueMetrics forQueue(String queueName, ms.register(sourceName(queueName).toString(), "Metrics for queue: " + queueName, metrics); } - queueMetrics.put(queueName, metrics); + QueueMetrics.getQueueMetrics().put(queueName, metrics); } return (CSQueueMetrics) metrics; 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/capacity/CapacitySchedulerQueueManager.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/CapacitySchedulerQueueManager.java index be6243d858..e33fbb33e2 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/CapacitySchedulerQueueManager.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/CapacitySchedulerQueueManager.java @@ -69,9 +69,11 @@ public class CapacitySchedulerQueueManager implements SchedulerQueueManager< new Comparator() { @Override public int compare(CSQueue q1, CSQueue q2) { - if (q1.getUsedCapacity() < q2.getUsedCapacity()) { + int result = Float.compare(q1.getUsedCapacity(), + q2.getUsedCapacity()); + if (result < 0) { return -1; - } else if (q1.getUsedCapacity() > q2.getUsedCapacity()) { + } else if (result > 0) { return 1; } 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/FSQueueMetrics.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/FSQueueMetrics.java index 22306a0f4a..4fe3973f7f 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/FSQueueMetrics.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/FSQueueMetrics.java @@ -206,7 +206,7 @@ static FSQueueMetrics forQueue(String queueName, Queue parent, public synchronized static FSQueueMetrics forQueue(MetricsSystem ms, String queueName, Queue parent, boolean enableUserMetrics, Configuration conf) { - QueueMetrics metrics = queueMetrics.get(queueName); + QueueMetrics metrics = QueueMetrics.getQueueMetrics().get(queueName); if (metrics == null) { metrics = new FSQueueMetrics(ms, queueName, parent, enableUserMetrics, conf) .tag(QUEUE_INFO, queueName); @@ -217,7 +217,7 @@ static FSQueueMetrics forQueue(MetricsSystem ms, String queueName, sourceName(queueName).toString(), "Metrics for queue: " + queueName, metrics); } - queueMetrics.put(queueName, metrics); + QueueMetrics.getQueueMetrics().put(queueName, metrics); } return (FSQueueMetrics)metrics; 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/FSSchedulerNode.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/FSSchedulerNode.java index 663e3c8b3a..6575e0c3ca 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/FSSchedulerNode.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/FSSchedulerNode.java @@ -155,10 +155,10 @@ synchronized LinkedHashMap getPreemptionList() { * Remove apps that have their preemption requests fulfilled. */ private synchronized void cleanupPreemptionList() { - Iterator iterator = - resourcesPreemptedForApp.keySet().iterator(); - while (iterator.hasNext()) { - FSAppAttempt app = iterator.next(); + Iterator> iterator = + resourcesPreemptedForApp.entrySet().iterator(); + while(iterator.hasNext()) { + FSAppAttempt app = iterator.next().getKey(); if (app.isStopped() || !app.isStarved()) { // App does not need more resources Resources.subtractFrom(totalResourcesPreempted,