Browse Source

YARN-536. Removed the unused objects ContainerStatus and ContainerStatus from Container which also don't belong to the container. Contributed by Xuan Gong.

git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/trunk@1464271 13f79535-47bb-0310-9956-ffa450edef68
Vinod Kumar Vavilapalli 12 years ago
parent
commit
1fd462b118

+ 3 - 0
hadoop-yarn-project/CHANGES.txt

@@ -69,6 +69,9 @@ Release 2.0.5-beta - UNRELEASED
 
     YARN-440. Flatten RegisterNodeManagerResponse. (Xuan Gong via sseth)
 
+    YARN-536. Removed the unused objects ContainerStatus and ContainerStatus from
+    Container which also don't belong to the container. (Xuan Gong via vinodkv)
+
   NEW FEATURES
 
   IMPROVEMENTS

+ 0 - 24
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/api/records/Container.java

@@ -124,18 +124,6 @@ public interface Container extends Comparable<Container> {
   @Unstable
   void setPriority(Priority priority);
   
-  /**
-   * Get the current <code>ContainerState</code> of the container.
-   * @return current <code>ContainerState</code> of the container
-   */
-  @Public
-  @Stable
-  ContainerState getState();
-  
-  @Private
-  @Unstable
-  void setState(ContainerState state);
-  
   /**
    * Get the <code>ContainerToken</code> for the container.
    * @return <code>ContainerToken</code> for the container
@@ -147,16 +135,4 @@ public interface Container extends Comparable<Container> {
   @Private
   @Unstable
   void setContainerToken(ContainerToken containerToken);
-  
-  /**
-   * Get the <code>ContainerStatus</code> of the container.
-   * @return <code>ContainerStatus</code> of the container
-   */
-  @Public
-  @Stable
-  ContainerStatus getContainerStatus();
-  
-  @Private
-  @Unstable
-  void setContainerStatus(ContainerStatus containerStatus);
 }

+ 1 - 79
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/api/records/impl/pb/ContainerPBImpl.java

@@ -21,8 +21,6 @@ package org.apache.hadoop.yarn.api.records.impl.pb;
 import org.apache.hadoop.security.proto.SecurityProtos.TokenProto;
 import org.apache.hadoop.yarn.api.records.Container;
 import org.apache.hadoop.yarn.api.records.ContainerId;
-import org.apache.hadoop.yarn.api.records.ContainerState;
-import org.apache.hadoop.yarn.api.records.ContainerStatus;
 import org.apache.hadoop.yarn.api.records.ContainerToken;
 import org.apache.hadoop.yarn.api.records.NodeId;
 import org.apache.hadoop.yarn.api.records.Priority;
@@ -31,12 +29,9 @@ import org.apache.hadoop.yarn.api.records.Resource;
 import org.apache.hadoop.yarn.proto.YarnProtos.ContainerIdProto;
 import org.apache.hadoop.yarn.proto.YarnProtos.ContainerProto;
 import org.apache.hadoop.yarn.proto.YarnProtos.ContainerProtoOrBuilder;
-import org.apache.hadoop.yarn.proto.YarnProtos.ContainerStateProto;
-import org.apache.hadoop.yarn.proto.YarnProtos.ContainerStatusProto;
 import org.apache.hadoop.yarn.proto.YarnProtos.NodeIdProto;
 import org.apache.hadoop.yarn.proto.YarnProtos.PriorityProto;
 import org.apache.hadoop.yarn.proto.YarnProtos.ResourceProto;
-import org.apache.hadoop.yarn.util.ProtoUtils;
     
 public class ContainerPBImpl extends ProtoBase<ContainerProto> implements Container {
 
@@ -49,7 +44,6 @@ public class ContainerPBImpl extends ProtoBase<ContainerProto> implements Contai
   private Resource resource = null;
   private Priority priority = null;
   private ContainerToken containerToken = null;
-  private ContainerStatus containerStatus = null;
   
   public ContainerPBImpl() {
     builder = ContainerProto.newBuilder();
@@ -94,11 +88,6 @@ public class ContainerPBImpl extends ProtoBase<ContainerProto> implements Contai
             builder.getContainerToken())) {
       builder.setContainerToken(convertToProtoFormat(this.containerToken));
     }
-    if (this.containerStatus != null
-        && !((ContainerStatusPBImpl) this.containerStatus).getProto().equals(
-            builder.getContainerStatus())) {
-      builder.setContainerStatus(convertToProtoFormat(this.containerStatus));
-    }
   }
 
   private void mergeLocalToProto() {
@@ -115,26 +104,7 @@ public class ContainerPBImpl extends ProtoBase<ContainerProto> implements Contai
     }
     viaProto = false;
   }
-    
-  
-  @Override
-  public ContainerState getState() {
-    ContainerProtoOrBuilder p = viaProto ? proto : builder;
-    if (!p.hasState()) {
-      return null;
-    }
-    return convertFromProtoFormat(p.getState());
-  }
 
-  @Override
-  public void setState(ContainerState state) {
-    maybeInitBuilder();
-    if (state == null) {
-      builder.clearState();
-      return;
-    }
-    builder.setState(convertToProtoFormat(state));
-  }
   @Override
   public ContainerId getId() {
     ContainerProtoOrBuilder p = viaProto ? proto : builder;
@@ -260,35 +230,6 @@ public class ContainerPBImpl extends ProtoBase<ContainerProto> implements Contai
     this.containerToken = containerToken;
   }
 
-  @Override
-  public ContainerStatus getContainerStatus() {
-    ContainerProtoOrBuilder p = viaProto ? proto : builder;
-    if (this.containerStatus != null) {
-      return this.containerStatus;
-    }
-    if (!p.hasContainerStatus()) {
-      return null;
-    }
-    this.containerStatus = convertFromProtoFormat(p.getContainerStatus());
-    return this.containerStatus;
-  }
-
-  @Override
-  public void setContainerStatus(ContainerStatus containerStatus) {
-    maybeInitBuilder();
-    if (containerStatus == null) 
-      builder.clearContainerStatus();
-    this.containerStatus = containerStatus;
-  }
-
-  private ContainerStateProto convertToProtoFormat(ContainerState e) {
-    return ProtoUtils.convertToProtoFormat(e);
-  }
-
-  private ContainerState convertFromProtoFormat(ContainerStateProto e) {
-    return ProtoUtils.convertFromProtoFormat(e);
-  }
-
   private ContainerIdPBImpl convertFromProtoFormat(ContainerIdProto p) {
     return new ContainerIdPBImpl(p);
   }
@@ -329,14 +270,6 @@ public class ContainerPBImpl extends ProtoBase<ContainerProto> implements Contai
     return ((ContainerTokenPBImpl)t).getProto();
   }
 
-  private ContainerStatusPBImpl convertFromProtoFormat(ContainerStatusProto p) {
-    return new ContainerStatusPBImpl(p);
-  }
-
-  private ContainerStatusProto convertToProtoFormat(ContainerStatus t) {
-    return ((ContainerStatusPBImpl)t).getProto();
-  }
-
   public String toString() {
     StringBuilder sb = new StringBuilder();
     sb.append("Container: [");
@@ -345,9 +278,7 @@ public class ContainerPBImpl extends ProtoBase<ContainerProto> implements Contai
     sb.append("NodeHttpAddress: ").append(getNodeHttpAddress()).append(", ");
     sb.append("Resource: ").append(getResource()).append(", ");
     sb.append("Priority: ").append(getPriority()).append(", ");
-    sb.append("State: ").append(getState()).append(", ");
     sb.append("Token: ").append(getContainerToken()).append(", ");
-    sb.append("Status: ").append(getContainerStatus());
     sb.append("]");
     return sb.toString();
   }
@@ -357,16 +288,7 @@ public class ContainerPBImpl extends ProtoBase<ContainerProto> implements Contai
   public int compareTo(Container other) {
     if (this.getId().compareTo(other.getId()) == 0) {
       if (this.getNodeId().compareTo(other.getNodeId()) == 0) {
-        if (this.getResource().compareTo(other.getResource()) == 0) {
-          if (this.getState().compareTo(other.getState()) == 0) {
-            //ContainerToken
-            return this.getState().compareTo(other.getState());
-          } else {
-            return this.getState().compareTo(other.getState());
-          }
-        } else {
-          return this.getResource().compareTo(other.getResource());
-        }
+        return this.getResource().compareTo(other.getResource());
       } else {
         return this.getNodeId().compareTo(other.getNodeId());
       }

+ 1 - 3
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/proto/yarn_protos.proto

@@ -67,9 +67,7 @@ message ContainerProto {
   optional string node_http_address = 3;
   optional ResourceProto resource = 4;
   optional PriorityProto priority = 5;
-  optional ContainerStateProto state = 6;
-  optional hadoop.common.TokenProto container_token = 7;
-  optional ContainerStatusProto container_status = 8;
+  optional hadoop.common.TokenProto container_token = 6;
 }
 
 enum YarnApplicationStateProto {

+ 0 - 1
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-applications-distributedshell/src/main/java/org/apache/hadoop/yarn/applications/distributedshell/ApplicationMaster.java

@@ -608,7 +608,6 @@ public class ApplicationMaster {
             + ", containerNode=" + allocatedContainer.getNodeId().getHost()
             + ":" + allocatedContainer.getNodeId().getPort()
             + ", containerNodeURI=" + allocatedContainer.getNodeHttpAddress()
-            + ", containerState" + allocatedContainer.getState()
             + ", containerResourceMemory"
             + allocatedContainer.getResource().getMemory());
         // + ", containerToken"

+ 0 - 5
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/util/BuilderUtils.java

@@ -246,11 +246,6 @@ public class BuilderUtils {
     container.setNodeHttpAddress(nodeHttpAddress);
     container.setResource(resource);
     container.setPriority(priority);
-    container.setState(ContainerState.NEW);
-    ContainerStatus containerStatus = Records.newRecord(ContainerStatus.class);
-    containerStatus.setContainerId(containerId);
-    containerStatus.setState(ContainerState.NEW);
-    container.setContainerStatus(containerStatus);
     container.setContainerToken(containerToken);
     return container;
   }

+ 1 - 6
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/rmcontainer/RMContainerImpl.java

@@ -288,14 +288,9 @@ public class RMContainerImpl implements RMContainer {
     public void transition(RMContainerImpl container, RMContainerEvent event) {
       RMContainerFinishedEvent finishedEvent = (RMContainerFinishedEvent) event;
 
-      // Update container-status for diagnostics. Today we completely
-      // replace it on finish. We may just need to update diagnostics.
-      container.container.setContainerStatus(finishedEvent
-          .getRemoteContainerStatus());
-
       // Inform AppAttempt
       container.eventHandler.handle(new RMAppAttemptContainerFinishedEvent(
-          container.appAttemptId, container.container.getContainerStatus()));
+          container.appAttemptId, finishedEvent.getRemoteContainerStatus()));
     }
   }
 

+ 3 - 4
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/MockNM.java

@@ -26,7 +26,6 @@ import java.util.Map;
 
 import org.apache.hadoop.yarn.api.records.ApplicationAttemptId;
 import org.apache.hadoop.yarn.api.records.ApplicationId;
-import org.apache.hadoop.yarn.api.records.Container;
 import org.apache.hadoop.yarn.api.records.ContainerState;
 import org.apache.hadoop.yarn.api.records.ContainerStatus;
 import org.apache.hadoop.yarn.api.records.NodeHealthStatus;
@@ -71,11 +70,11 @@ public class MockNM {
     this.resourceTracker = resourceTracker;
   }
 
-  public void containerStatus(Container container) throws Exception {
+  public void containerStatus(ContainerStatus containerStatus) throws Exception {
     Map<ApplicationId, List<ContainerStatus>> conts = 
         new HashMap<ApplicationId, List<ContainerStatus>>();
-    conts.put(container.getId().getApplicationAttemptId().getApplicationId(), 
-        Arrays.asList(new ContainerStatus[] { container.getContainerStatus() }));
+    conts.put(containerStatus.getContainerId().getApplicationAttemptId().getApplicationId(),
+        Arrays.asList(new ContainerStatus[] { containerStatus }));
     nodeHeartbeat(conts, true);
   }
 

+ 13 - 5
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/NodeManager.java

@@ -76,6 +76,9 @@ public class NodeManager implements ContainerManager {
   final Map<ApplicationId, List<Container>> containers = 
     new HashMap<ApplicationId, List<Container>>();
   
+  final Map<Container, ContainerStatus> containerStatusMap =
+      new HashMap<Container, ContainerStatus>();
+
   public NodeManager(String hostName, int containerManagerPort, int httpPort,
       String rackName, Resource capability,
       ResourceTrackerService resourceTrackerService, RMContext rmContext)
@@ -137,7 +140,7 @@ public class NodeManager implements ContainerManager {
     List<ContainerStatus> containerStatuses = new ArrayList<ContainerStatus>();
     for (List<Container> appContainers : containers.values()) {
       for (Container container : appContainers) {
-        containerStatuses.add(container.getContainerStatus());
+        containerStatuses.add(containerStatusMap.get(container));
       }
     }
     return containerStatuses;
@@ -189,8 +192,11 @@ public class NodeManager implements ContainerManager {
             null, null                                 // DKDC - Doesn't matter
             );
 
+    ContainerStatus containerStatus =
+        BuilderUtils.newContainerStatus(container.getId(), ContainerState.NEW,
+            "", -1000);
     applicationContainers.add(container);
-    
+    containerStatusMap.put(container, containerStatus);
     Resources.subtractFrom(available, containerLaunchContext.getResource());
     Resources.addTo(used, containerLaunchContext.getResource());
     
@@ -223,7 +229,9 @@ public class NodeManager implements ContainerManager {
     List<Container> applicationContainers = containers.get(applicationId);
     for (Container c : applicationContainers) {
       if (c.getId().compareTo(containerID) == 0) {
-        c.setState(ContainerState.COMPLETE);
+        ContainerStatus containerStatus = containerStatusMap.get(c);
+        containerStatus.setState(ContainerState.COMPLETE);
+        containerStatusMap.put(c, containerStatus);
       }
     }
     
@@ -277,8 +285,8 @@ public class NodeManager implements ContainerManager {
     }
     GetContainerStatusResponse response = 
         recordFactory.newRecordInstance(GetContainerStatusResponse.class);
-    if (container != null && container.getContainerStatus() != null) {
-      response.setStatus(container.getContainerStatus());
+    if (container != null && containerStatusMap.get(container).getState() != null) {
+      response.setStatus(containerStatusMap.get(container));
     }
     return response;
   }

+ 3 - 2
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/TestFifoScheduler.java

@@ -139,8 +139,9 @@ public class TestFifoScheduler {
 
     Container c1 = allocated1.get(0);
     Assert.assertEquals(GB, c1.getResource().getMemory());
-    c1.setState(ContainerState.COMPLETE);
-    nm1.containerStatus(c1);
+    ContainerStatus containerStatus = BuilderUtils.newContainerStatus(
+        c1.getId(), ContainerState.COMPLETE, "", 0);
+    nm1.containerStatus(containerStatus);
     int waitCount = 0;
     while (attempt1.getJustFinishedContainers().size() < 1
         && waitCount++ != 20) {