From dc8ad1213b37f92340e1ef161af809344a38cc2c Mon Sep 17 00:00:00 2001 From: Bikas Saha Date: Mon, 15 Jul 2013 17:18:47 +0000 Subject: [PATCH] Merge r1503353 from trunk to branch-2 for 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/branches/branch-2@1503354 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 c599ae81313..8e00acac8e9 100644 --- a/hadoop-yarn-project/CHANGES.txt +++ b/hadoop-yarn-project/CHANGES.txt @@ -672,6 +672,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 bd0f16b63e7..e0f9fad6b1c 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.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 static class ContainerRequest { 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 467ab31744d..75ff744f15c 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 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 @@ protected void serviceStop() throws Exception { 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 RegisterApplicationMasterResponse registerApplicationMaster( @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 @@ protected void populateNMTokens(AllocateResponse allocateResponse) { 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 void unregisterApplicationMaster(FinalApplicationStatus appStatus, @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 synchronized void addContainerRequest(T req) { @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 synchronized void removeContainerRequest(T req) { @Override public synchronized void releaseAssignedContainer(ContainerId containerId) { + Preconditions.checkArgument(containerId != null, + "ContainerId can not be null."); release.add(containerId); } @@ -398,6 +411,10 @@ public synchronized List> getMatchingRequests( 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);