Patch for findbugs warnings in Resource Manager (siddharth seth via mahadev)

git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/branches/MR-279@1155161 13f79535-47bb-0310-9956-ffa450edef68

mapreduce/CHANGES.txt

@@ -3,7 +3,12 @@ Hadoop MapReduce Change Log
 Trunk (unreleased changes)
 
   MAPREDUCE-279
-
+    
+    Patch for findbugs warnings in Resource Manager (siddharth seth via mahadev)
+    
+    Fix ivy conf to work with the hadoop common trunk maven build changes.
+    (Giridharan Kesavan)
+    
     Fix the poms to enable 0.23 snapshots for hdfs/common from apache 
     nightly builds (gkesavan)
  
@@ -2456,6 +2461,4 @@ Release 0.21.0 - 2010-08-13
 
     MAPREDUCE-1856. Extract a subset of tests for smoke (DOA) validation (cos)
     
-    Fix ivy conf to work with the hadoop common trunk maven build changes.
-    (Giridharan Kesavan)
 

mapreduce/yarn/dev-support/findbugs-exclude.xml

@@ -112,6 +112,35 @@
     <Class name="org.apache.hadoop.yarn.server.nodemanager.containermanager.logaggregation.LogAggregationService" />
     <Bug pattern="IS2_INCONSISTENT_SYNC" />
   </Match>
+  <!-- Inconsistent sync warning - reinitialize read from other queue does not need sync-->
+  <Match>
+    <Class name="org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.LeafQueue" />
+    <Or>
+      <Field name="absoluteCapacity" />
+      <Field name="absoluteMaxCapacity" />
+      <Field name="acls" />
+      <Field name="capacity" />
+      <Field name="maxApplications" />
+      <Field name="maxApplicationsPerUser" />
+      <Field name="maximumCapacity" />
+      <Field name="state" />
+      <Field name="userLimit" />
+      <Field name="userLimitFactor" />
+    </Or>
+    <Bug pattern="IS2_INCONSISTENT_SYNC" />
+  </Match>
+  <Match>
+    <Class name="org.apache.hadoop.yarn.server.resourcemanager.scheduler.capacity.ParentQueue" />
+    <Or>
+      <Field name="absoluteCapacity" />
+      <Field name="absoluteMaxCapacity" />
+      <Field name="acls" />
+      <Field name="capacity" />
+      <Field name="maximumCapacity" />
+      <Field name="state" />
+    </Or>
+    <Bug pattern="IS2_INCONSISTENT_SYNC" />
+  </Match>
 
   <!-- Don't care if putIfAbsent value is ignored -->
   <Match>
@@ -128,5 +157,27 @@
     <Class name="org.apache.hadoop.yarn.webapp.WebApps$Builder" />
     <Bug pattern="DM_EXIT" />
   </Match>
+  <!-- AsyncDispatcher will kill the process if there is an error dispatching -->
+  <Match>
+    <Class name="org.apache.hadoop.yarn.event.AsyncDispatcher" />
+    <Method name="dispatch" />
+    <Bug pattern="DM_EXIT" />
+  </Match>
 
- </FindBugsFilter>
+  <!-- Ignore heartbeat exception when killing localizer -->
+  <Match>
+    <Class name="org.apache.hadoop.yarn.server.nodemanager.containermanager.localizer.ContainerLocalizer" />
+    <Method name="localizeFiles" />
+    <Bug pattern="DE_MIGHT_IGNORE" />
+  </Match>
+
+  <!-- Ignore EI_EXPOSE_REP2 in Log services -->
+  <Match>
+    <Class name="org.apache.hadoop.yarn.server.nodemanager.containermanager.logaggregation.AggregatedLogFormat$LogValue" />
+    <Bug pattern="EI_EXPOSE_REP2" />
+  </Match>
+  <Match>
+    <Class name="org.apache.hadoop.yarn.server.nodemanager.containermanager.logaggregation.AppLogAggregatorImpl" />
+    <Bug pattern="EI_EXPOSE_REP2" />
+  </Match>
+</FindBugsFilter>

mapreduce/yarn/yarn-api/src/main/java/org/apache/hadoop/yarn/api/records/impl/pb/ApplicationAttemptIdPBImpl.java

@@ -114,6 +114,7 @@ public class ApplicationAttemptIdPBImpl extends ProtoBase<ApplicationAttemptIdPr
 
   @Override
   public synchronized boolean equals(Object other) {
+    if (other == null) return false;
     if (other.getClass().isAssignableFrom(this.getClass())) {
       return this.getProto().equals(this.getClass().cast(other).getProto());
     }

mapreduce/yarn/yarn-api/src/main/java/org/apache/hadoop/yarn/api/records/impl/pb/ApplicationReportPBImpl.java

@@ -3,13 +3,11 @@ package org.apache.hadoop.yarn.api.records.impl.pb;
 import org.apache.hadoop.yarn.api.records.ApplicationId;
 import org.apache.hadoop.yarn.api.records.ApplicationReport;
 import org.apache.hadoop.yarn.api.records.ApplicationState;
-import org.apache.hadoop.yarn.api.records.Container;
 import org.apache.hadoop.yarn.api.records.ProtoBase;
 import org.apache.hadoop.yarn.proto.YarnProtos.ApplicationIdProto;
 import org.apache.hadoop.yarn.proto.YarnProtos.ApplicationReportProto;
 import org.apache.hadoop.yarn.proto.YarnProtos.ApplicationReportProtoOrBuilder;
 import org.apache.hadoop.yarn.proto.YarnProtos.ApplicationStateProto;
-import org.apache.hadoop.yarn.proto.YarnProtos.ContainerProto;
 import org.apache.hadoop.yarn.util.ProtoUtils;
 
 public class ApplicationReportPBImpl extends ProtoBase<ApplicationReportProto> 
@@ -263,9 +261,4 @@ implements ApplicationReport {
       ApplicationIdProto applicationId) {
     return new ApplicationIdPBImpl(applicationId);
   }
-
-  private Container convertFromProtoFormat(ContainerProto c) {
-    return new ContainerPBImpl(c);
-  }
-
 }

mapreduce/yarn/yarn-common/src/main/java/org/apache/hadoop/yarn/event/AsyncDispatcher.java

@@ -113,6 +113,7 @@ public class AsyncDispatcher extends AbstractService implements Dispatcher {
       eventDispatchers.get(type).handle(event);
     }
     catch (Throwable t) {
+      //TODO Maybe log the state of the queue
       LOG.fatal("Error in dispatcher thread. Exiting..", t);
       System.exit(-1);
     }

mapreduce/yarn/yarn-common/src/main/java/org/apache/hadoop/yarn/ipc/ProtoOverHadoopRpcEngine.java

@@ -82,7 +82,7 @@ public class ProtoOverHadoopRpcEngine implements RpcEngine {
     }
   }
 
-  private class Invoker implements InvocationHandler, Closeable {
+  private static class Invoker implements InvocationHandler, Closeable {
     private Map<String, Message> returnTypes = new ConcurrentHashMap<String, Message>();
     private boolean isClosed = false;
     private Client.ConnectionId remoteId;

mapreduce/yarn/yarn-common/src/main/java/org/apache/hadoop/yarn/util/AbstractLivelinessMonitor.java

@@ -35,7 +35,7 @@ public abstract class AbstractLivelinessMonitor<O> extends AbstractService {
   //received.
   private Thread checkerThread;
   private volatile boolean stopped;
-  public static int DEFAULT_EXPIRE = 5*60*1000;//5 mins
+  public static final int DEFAULT_EXPIRE = 5*60*1000;//5 mins
   private int expireInterval = DEFAULT_EXPIRE;
   private int monitorInterval = expireInterval/3;
 

mapreduce/yarn/yarn-server/yarn-server-common/src/main/java/org/apache/hadoop/yarn/server/api/records/impl/pb/NodeStatusPBImpl.java

@@ -26,7 +26,6 @@ import org.apache.hadoop.yarn.proto.YarnServerCommonProtos.ContainerListProto;
 import org.apache.hadoop.yarn.proto.YarnServerCommonProtos.NodeStatusProto;
 import org.apache.hadoop.yarn.proto.YarnServerCommonProtos.NodeStatusProtoOrBuilder;
 import org.apache.hadoop.yarn.server.api.records.NodeStatus;
-import org.mortbay.util.ajax.JSON.Convertor;
     
 public class NodeStatusPBImpl extends ProtoBase<NodeStatusProto> implements NodeStatus {
   NodeStatusProto proto = NodeStatusProto.getDefaultInstance();
@@ -267,7 +266,7 @@ public class NodeStatusPBImpl extends ProtoBase<NodeStatusProto> implements Node
   @Override
   public void removeContainers(ApplicationId applicationId) {
     initContainers();
-    this.containers.remove(applicationId);
+    this.containers.remove(convertToProtoFormat(applicationId));
   }
   
   @Override

mapreduce/yarn/yarn-server/yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/NodeStatusUpdaterImpl.java

@@ -196,35 +196,33 @@ public class NodeStatusUpdaterImpl extends AbstractService implements
     nodeStatus.setNodeId(this.nodeId);
 
     int numActiveContainers = 0;
-    synchronized (this.context.getContainers()) {
-      for (Iterator<Entry<ContainerId, Container>> i =
-          this.context.getContainers().entrySet().iterator(); i.hasNext();) {
-        Entry<ContainerId, Container> e = i.next();
-        ContainerId containerId = e.getKey();
-        Container container = e.getValue();
+    for (Iterator<Entry<ContainerId, Container>> i =
+        this.context.getContainers().entrySet().iterator(); i.hasNext();) {
+      Entry<ContainerId, Container> e = i.next();
+      ContainerId containerId = e.getKey();
+      Container container = e.getValue();
 
-        List<org.apache.hadoop.yarn.api.records.Container> applicationContainers = nodeStatus
-            .getContainers(container.getContainerID().getAppId());
-        if (applicationContainers == null) {
-          applicationContainers = new ArrayList<org.apache.hadoop.yarn.api.records.Container>();
-          nodeStatus.setContainers(container.getContainerID().getAppId(),
-              applicationContainers);
-        }
+      List<org.apache.hadoop.yarn.api.records.Container> applicationContainers = nodeStatus
+          .getContainers(container.getContainerID().getAppId());
+      if (applicationContainers == null) {
+        applicationContainers = new ArrayList<org.apache.hadoop.yarn.api.records.Container>();
+        nodeStatus.setContainers(container.getContainerID().getAppId(),
+            applicationContainers);
+      }
 
-        // Clone the container to send it to the RM
-        org.apache.hadoop.yarn.api.records.Container c = container.cloneAndGetContainer();
-        c.setNodeId(this.nodeId);
-        c.setNodeHttpAddress(this.nodeHttpAddress); // TODO: don't set everytime.
-        applicationContainers.add(c);
-        ++numActiveContainers;
-        LOG.info("Sending out status for container: " + c);
+      // Clone the container to send it to the RM
+      org.apache.hadoop.yarn.api.records.Container c = container.cloneAndGetContainer();
+      c.setNodeId(this.nodeId);
+      c.setNodeHttpAddress(this.nodeHttpAddress); // TODO: don't set everytime.
+      applicationContainers.add(c);
+      ++numActiveContainers;
+      LOG.info("Sending out status for container: " + c);
 
-        if (c.getState() == ContainerState.COMPLETE) {
-          // Remove
-          i.remove();
+      if (c.getState() == ContainerState.COMPLETE) {
+        // Remove
+        i.remove();
 
-          LOG.info("Removed completed container " + containerId);
-        }
+        LOG.info("Removed completed container " + containerId);
       }
     }
 

mapreduce/yarn/yarn-server/yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/logaggregation/AggregatedLogFormat.java

@@ -107,11 +107,16 @@ public class AggregatedLogFormat {
           out.writeUTF(String.valueOf(logFile.length()));
 
           // Write the log itself
-          FileInputStream in = new FileInputStream(logFile);
-          byte[] buf = new byte[65535];
-          int len = 0;
-          while ((len = in.read(buf)) != -1) {
-            out.write(buf, 0, len);
+          FileInputStream in = null;
+          try {
+            in = new FileInputStream(logFile);
+            byte[] buf = new byte[65535];
+            int len = 0;
+            while ((len = in.read(buf)) != -1) {
+              out.write(buf, 0, len);
+            }
+          } finally {
+            in.close();
           }
         }
       }

mapreduce/yarn/yarn-server/yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/webapp/ContainerLogsPage.java

@@ -141,6 +141,7 @@ public class ContainerLogsPage extends NMView {
               totalRead += len;
               toRead = toRead - totalRead;
             }
+            reader.close();
             writer().write("</pre>");
           } catch (IOException e) {
               writer().write(

mapreduce/yarn/yarn-server/yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/AppSchedulingInfo.java

@@ -325,7 +325,7 @@ public class AppSchedulingInfo {
     clearRequests();
   }
 
-  public void setQueue(Queue queue) {
+  public synchronized void setQueue(Queue queue) {
     this.queue = queue;
   }
 }

mapreduce/yarn/yarn-server/yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/LeafQueue.java

@@ -203,24 +203,24 @@ public class LeafQueue implements Queue {
         ", state=" + state +
         ", acls=" + aclsString);
   }
-
+  
   @Override
-  public float getCapacity() {
+  public synchronized float getCapacity() {
     return capacity;
   }
 
   @Override
-  public float getAbsoluteCapacity() {
+  public synchronized float getAbsoluteCapacity() {
     return absoluteCapacity;
   }
 
   @Override
-  public float getMaximumCapacity() {
+  public synchronized float getMaximumCapacity() {
     return maximumCapacity;
   }
 
   @Override
-  public float getAbsoluteMaximumCapacity() {
+  public synchronized float getAbsoluteMaximumCapacity() {
     return absoluteMaxCapacity;
   }
 
@@ -240,7 +240,7 @@ public class LeafQueue implements Queue {
   }
 
   @Override
-  public float getUsedCapacity() {
+  public synchronized float getUsedCapacity() {
     return usedCapacity;
   }
 
@@ -271,12 +271,12 @@ public class LeafQueue implements Queue {
     return applications.size();
   }
 
-  public int getNumContainers() {
+  public synchronized int getNumContainers() {
     return numContainers;
   }
 
   @Override
-  public QueueState getState() {
+  public synchronized QueueState getState() {
     return state;
   }
 

mapreduce/yarn/yarn-server/yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/capacity/ParentQueue.java

@@ -216,12 +216,12 @@ public class ParentQueue implements Queue {
   }
 
   @Override
-  public float getCapacity() {
+  public synchronized float getCapacity() {
     return capacity;
   }
 
   @Override
-  public float getAbsoluteCapacity() {
+  public synchronized float getAbsoluteCapacity() {
     return absoluteCapacity;
   }
 
@@ -236,7 +236,7 @@ public class ParentQueue implements Queue {
   }
 
   @Override
-  public float getUsedCapacity() {
+  public synchronized float getUsedCapacity() {
     return usedCapacity;
   }
 
@@ -255,21 +255,21 @@ public class ParentQueue implements Queue {
     return new ArrayList<Queue>(childQueues);
   }
 
-  public int getNumContainers() {
+  public synchronized int getNumContainers() {
     return numContainers;
   }
   
-  public int getNumApplications() {
+  public synchronized int getNumApplications() {
     return numApplications;
   }
 
   @Override
-  public QueueState getState() {
+  public synchronized QueueState getState() {
     return state;
   }
 
   @Override
-  public Map<QueueACL, AccessControlList> getQueueAcls() {
+  public synchronized Map<QueueACL, AccessControlList> getQueueAcls() {
     return new HashMap<QueueACL, AccessControlList>(acls);
   }