From fe735f237c735a375d2bd194ed80ef9949fb1a68 Mon Sep 17 00:00:00 2001 From: Bikas Saha Date: Mon, 15 Jul 2013 17:13:05 +0000 Subject: [PATCH] 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 | 3 +++ .../hadoop/yarn/client/api/AMRMClient.java | 11 +++++++++- .../yarn/client/api/impl/AMRMClientImpl.java | 21 +++++++++++++++++-- 3 files changed, 32 insertions(+), 3 deletions(-) diff --git a/hadoop-yarn-project/CHANGES.txt b/hadoop-yarn-project/CHANGES.txt index 410e4eed1e..7cd9e2f2e4 100644 --- a/hadoop-yarn-project/CHANGES.txt +++ b/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. diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/main/java/org/apache/hadoop/yarn/client/api/AMRMClient.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/main/java/org/apache/hadoop/yarn/client/api/AMRMClient.java index bd0f16b63e..e0f9fad6b1 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/main/java/org/apache/hadoop/yarn/client/api/AMRMClient.java +++ b/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 extends @Public public static AMRMClient createAMRMClient( ApplicationAttemptId appAttemptId) { + Preconditions.checkArgument(appAttemptId != null, + "ApplicationAttempId should not be null"); AMRMClient client = new AMRMClientImpl(appAttemptId); return client; } @@ -95,6 +97,13 @@ public abstract class AMRMClient 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); diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/main/java/org/apache/hadoop/yarn/client/api/impl/AMRMClientImpl.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/main/java/org/apache/hadoop/yarn/client/api/impl/AMRMClientImpl.java index 467ab31744..75ff744f15 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/main/java/org/apache/hadoop/yarn/client/api/impl/AMRMClientImpl.java +++ b/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 extends AMRMClient { 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 extends AMRMClient { @Override public AllocateResponse allocate(float progressIndicator) throws YarnException, IOException { + Preconditions.checkArgument(progressIndicator > 0, + "Progress indicator should not be negative"); AllocateResponse allocateResponse = null; ArrayList askList = null; ArrayList releaseList = null; @@ -302,6 +307,8 @@ public class AMRMClientImpl extends AMRMClient { 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 extends AMRMClient { @Override public synchronized void addContainerRequest(T req) { + Preconditions.checkArgument(req != null, + "Resource request can not be null."); Set allRacks = new HashSet(); if (req.getRacks() != null) { allRacks.addAll(req.getRacks()); @@ -355,6 +364,8 @@ public class AMRMClientImpl extends AMRMClient { @Override public synchronized void removeContainerRequest(T req) { + Preconditions.checkArgument(req != null, + "Resource request can not be null."); Set allRacks = new HashSet(); if (req.getRacks() != null) { allRacks.addAll(req.getRacks()); @@ -380,6 +391,8 @@ public class AMRMClientImpl extends AMRMClient { @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 extends AMRMClient { 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> list = new LinkedList>(); Map> remoteRequests = this.remoteRequestsTable.get(priority);