YARN-654. AMRMClient: Perform sanity checks for parameters of public methods (Xuan Gong via bikas)

git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/trunk@1503353 13f79535-47bb-0310-9956-ffa450edef68

hadoop-yarn-project/CHANGES.txt

@@ -689,6 +689,9 @@ Release 2.1.0-beta - 2013-07-02
     YARN-763. AMRMClientAsync should stop heartbeating after receiving
     shutdown from RM (Xuan Gong via bikas)
 
+    YARN-654. AMRMClient: Perform sanity checks for parameters of public
+    methods (Xuan Gong via bikas)"
+
   BREAKDOWN OF HADOOP-8562 SUBTASKS AND RELATED JIRAS
 
     YARN-158. Yarn creating package-info.java must not depend on sh.

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/main/java/org/apache/hadoop/yarn/client/api/AMRMClient.java

@@ -36,8 +36,8 @@ import org.apache.hadoop.yarn.api.records.Priority;
 import org.apache.hadoop.yarn.api.records.Resource;
 import org.apache.hadoop.yarn.client.api.impl.AMRMClientImpl;
 import org.apache.hadoop.yarn.exceptions.YarnException;
-
 import com.google.common.collect.ImmutableList;
+import com.google.common.base.Preconditions;
 
 @InterfaceAudience.Public
 @InterfaceStability.Stable
@@ -57,6 +57,8 @@ public abstract class AMRMClient<T extends AMRMClient.ContainerRequest> extends
   @Public
   public static <T extends ContainerRequest> AMRMClient<T> createAMRMClient(
       ApplicationAttemptId appAttemptId) {
+    Preconditions.checkArgument(appAttemptId != null,
+        "ApplicationAttempId should not be null");
     AMRMClient<T> client = new AMRMClientImpl<T>(appAttemptId);
     return client;
   }
@@ -95,6 +97,13 @@ public abstract class AMRMClient<T extends AMRMClient.ContainerRequest> extends
         
     public ContainerRequest(Resource capability, String[] nodes,
         String[] racks, Priority priority, int containerCount) {
+      Preconditions.checkArgument(capability != null,
+          "The Resource to be requested for each container " +
+              "should not be null ");
+      Preconditions.checkArgument(priority != null,
+          "The priority at which to request containers should not be null ");
+      Preconditions.checkArgument(containerCount > 0,
+          "The number of containers to request should larger than 0");
       this.capability = capability;
       this.nodes = (nodes != null ? ImmutableList.copyOf(nodes) : null);
       this.racks = (racks != null ? ImmutableList.copyOf(racks) : null);

hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/main/java/org/apache/hadoop/yarn/client/api/impl/AMRMClientImpl.java

@@ -68,8 +68,7 @@ import org.apache.hadoop.yarn.util.RackResolver;
 
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Joiner;
-
-// TODO check inputs for null etc. YARN-654
+import com.google.common.base.Preconditions;
 
 @Private
 @Unstable
@@ -204,6 +203,10 @@ public class AMRMClientImpl<T extends ContainerRequest> extends AMRMClient<T> {
   public RegisterApplicationMasterResponse registerApplicationMaster(
       String appHostName, int appHostPort, String appTrackingUrl)
       throws YarnException, IOException {
+    Preconditions.checkArgument(appHostName != null,
+        "The host name should not be null");
+    Preconditions.checkArgument(appHostPort >= 0,
+        "Port number of the host should not be negative");
     // do this only once ???
     RegisterApplicationMasterRequest request = recordFactory
         .newRecordInstance(RegisterApplicationMasterRequest.class);
@@ -223,6 +226,8 @@ public class AMRMClientImpl<T extends ContainerRequest> extends AMRMClient<T> {
   @Override
   public AllocateResponse allocate(float progressIndicator) 
       throws YarnException, IOException {
+    Preconditions.checkArgument(progressIndicator > 0,
+        "Progress indicator should not be negative");
     AllocateResponse allocateResponse = null;
     ArrayList<ResourceRequest> askList = null;
     ArrayList<ContainerId> releaseList = null;
@@ -302,6 +307,8 @@ public class AMRMClientImpl<T extends ContainerRequest> extends AMRMClient<T> {
   public void unregisterApplicationMaster(FinalApplicationStatus appStatus,
       String appMessage, String appTrackingUrl) throws YarnException,
       IOException {
+    Preconditions.checkArgument(appStatus != null,
+        "AppStatus should not be null.");
     FinishApplicationMasterRequest request = recordFactory
                   .newRecordInstance(FinishApplicationMasterRequest.class);
     request.setAppAttemptId(appAttemptId);
@@ -317,6 +324,8 @@ public class AMRMClientImpl<T extends ContainerRequest> extends AMRMClient<T> {
   
   @Override
   public synchronized void addContainerRequest(T req) {
+    Preconditions.checkArgument(req != null,
+        "Resource request can not be null.");
     Set<String> allRacks = new HashSet<String>();
     if (req.getRacks() != null) {
       allRacks.addAll(req.getRacks());
@@ -355,6 +364,8 @@ public class AMRMClientImpl<T extends ContainerRequest> extends AMRMClient<T> {
 
   @Override
   public synchronized void removeContainerRequest(T req) {
+    Preconditions.checkArgument(req != null,
+        "Resource request can not be null.");
     Set<String> allRacks = new HashSet<String>();
     if (req.getRacks() != null) {
       allRacks.addAll(req.getRacks());
@@ -380,6 +391,8 @@ public class AMRMClientImpl<T extends ContainerRequest> extends AMRMClient<T> {
 
   @Override
   public synchronized void releaseAssignedContainer(ContainerId containerId) {
+    Preconditions.checkArgument(containerId != null,
+        "ContainerId can not be null.");
     release.add(containerId);
   }
   
@@ -398,6 +411,10 @@ public class AMRMClientImpl<T extends ContainerRequest> extends AMRMClient<T> {
                                           Priority priority, 
                                           String resourceName, 
                                           Resource capability) {
+    Preconditions.checkArgument(capability != null,
+        "The Resource to be requested should not be null ");
+    Preconditions.checkArgument(priority != null,
+        "The priority at which to request containers should not be null ");
     List<LinkedHashSet<T>> list = new LinkedList<LinkedHashSet<T>>();
     Map<String, TreeMap<Resource, ResourceRequestInfo>> remoteRequests = 
         this.remoteRequestsTable.get(priority);