YARN-8412. Move ResourceRequest.clone logic everywhere into a proper API. Contributed by Botong Huang.
Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/99948565 Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/99948565 Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/99948565 Branch: refs/heads/HDDS-4 Commit: 99948565cb5d5706241d7a8fc591e1617c499e03 Parents: 59de967 Author: Inigo Goiri <inigo...@apache.org> Authored: Thu Jun 21 18:24:10 2018 -0700 Committer: Inigo Goiri <inigo...@apache.org> Committed: Thu Jun 21 18:24:10 2018 -0700 ---------------------------------------------------------------------- .../yarn/api/records/ResourceRequest.java | 20 ++++++++++++++++++++ .../yarn/client/api/impl/AMRMClientImpl.java | 11 +---------- .../api/impl/TestAMRMClientOnRMRestart.java | 6 +----- .../hadoop/yarn/server/AMRMClientRelayer.java | 8 +------- .../LocalityMulticastAMRMProxyPolicy.java | 10 +--------- .../server/scheduler/ResourceRequestSet.java | 14 ++------------ .../hadoop/yarn/server/utils/BuilderUtils.java | 12 ------------ .../LocalityAppPlacementAllocator.java | 10 ++-------- .../server/resourcemanager/Application.java | 10 ++++------ .../server/resourcemanager/TestAppManager.java | 18 ++---------------- 10 files changed, 34 insertions(+), 85 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hadoop/blob/99948565/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/api/records/ResourceRequest.java ---------------------------------------------------------------------- diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/api/records/ResourceRequest.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/api/records/ResourceRequest.java index eea81fe..a863910 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/api/records/ResourceRequest.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/api/records/ResourceRequest.java @@ -102,6 +102,26 @@ public abstract class ResourceRequest implements Comparable<ResourceRequest> { .build(); } + /** + * Clone a ResourceRequest object (shallow copy). Please keep it loaded with + * all (new) fields + * + * @param rr the object to copy from + * @return the copied object + */ + @Public + @Evolving + public static ResourceRequest clone(ResourceRequest rr) { + // Please keep it loaded with all (new) fields + return ResourceRequest.newBuilder().priority(rr.getPriority()) + .resourceName(rr.getResourceName()).capability(rr.getCapability()) + .numContainers(rr.getNumContainers()) + .relaxLocality(rr.getRelaxLocality()) + .nodeLabelExpression(rr.getNodeLabelExpression()) + .executionTypeRequest(rr.getExecutionTypeRequest()) + .allocationRequestId(rr.getAllocationRequestId()).build(); + } + @Public @Unstable public static ResourceRequestBuilder newBuilder() { http://git-wip-us.apache.org/repos/asf/hadoop/blob/99948565/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/main/java/org/apache/hadoop/yarn/client/api/impl/AMRMClientImpl.java ---------------------------------------------------------------------- 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 36c3cf1..7265d24 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 @@ -451,16 +451,7 @@ public class AMRMClientImpl<T extends ContainerRequest> extends AMRMClient<T> { for(ResourceRequest r : ask) { // create a copy of ResourceRequest as we might change it while the // RPC layer is using it to send info across - ResourceRequest rr = - ResourceRequest.newBuilder().priority(r.getPriority()) - .resourceName(r.getResourceName()).capability(r.getCapability()) - .numContainers(r.getNumContainers()) - .relaxLocality(r.getRelaxLocality()) - .nodeLabelExpression(r.getNodeLabelExpression()) - .executionTypeRequest(r.getExecutionTypeRequest()) - .allocationRequestId(r.getAllocationRequestId()) - .build(); - askList.add(rr); + askList.add(ResourceRequest.clone(r)); } return askList; } http://git-wip-us.apache.org/repos/asf/hadoop/blob/99948565/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/test/java/org/apache/hadoop/yarn/client/api/impl/TestAMRMClientOnRMRestart.java ---------------------------------------------------------------------- diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/test/java/org/apache/hadoop/yarn/client/api/impl/TestAMRMClientOnRMRestart.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/test/java/org/apache/hadoop/yarn/client/api/impl/TestAMRMClientOnRMRestart.java index 11d703d..5104866 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/test/java/org/apache/hadoop/yarn/client/api/impl/TestAMRMClientOnRMRestart.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/test/java/org/apache/hadoop/yarn/client/api/impl/TestAMRMClientOnRMRestart.java @@ -570,11 +570,7 @@ public class TestAMRMClientOnRMRestart { ContainerUpdates updateRequests) { List<ResourceRequest> askCopy = new ArrayList<ResourceRequest>(); for (ResourceRequest req : ask) { - ResourceRequest reqCopy = - ResourceRequest.newInstance(req.getPriority(), - req.getResourceName(), req.getCapability(), - req.getNumContainers(), req.getRelaxLocality()); - askCopy.add(reqCopy); + askCopy.add(ResourceRequest.clone(req)); } lastAsk = ask; lastRelease = release; http://git-wip-us.apache.org/repos/asf/hadoop/blob/99948565/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/main/java/org/apache/hadoop/yarn/server/AMRMClientRelayer.java ---------------------------------------------------------------------- diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/main/java/org/apache/hadoop/yarn/server/AMRMClientRelayer.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/main/java/org/apache/hadoop/yarn/server/AMRMClientRelayer.java index c216ace..e8a7f64 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/main/java/org/apache/hadoop/yarn/server/AMRMClientRelayer.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/main/java/org/apache/hadoop/yarn/server/AMRMClientRelayer.java @@ -220,13 +220,7 @@ public class AMRMClientRelayer extends AbstractService for (ResourceRequest r : ask) { // create a copy of ResourceRequest as we might change it while the // RPC layer is using it to send info across - askList.add(ResourceRequest.newBuilder().priority(r.getPriority()) - .resourceName(r.getResourceName()).capability(r.getCapability()) - .numContainers(r.getNumContainers()) - .relaxLocality(r.getRelaxLocality()) - .nodeLabelExpression(r.getNodeLabelExpression()) - .executionTypeRequest(r.getExecutionTypeRequest()) - .allocationRequestId(r.getAllocationRequestId()).build()); + askList.add(ResourceRequest.clone(r)); } allocateRequest = AllocateRequest.newBuilder() http://git-wip-us.apache.org/repos/asf/hadoop/blob/99948565/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/main/java/org/apache/hadoop/yarn/server/federation/policies/amrmproxy/LocalityMulticastAMRMProxyPolicy.java ---------------------------------------------------------------------- diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/main/java/org/apache/hadoop/yarn/server/federation/policies/amrmproxy/LocalityMulticastAMRMProxyPolicy.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/main/java/org/apache/hadoop/yarn/server/federation/policies/amrmproxy/LocalityMulticastAMRMProxyPolicy.java index d303d6f..1481f34 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/main/java/org/apache/hadoop/yarn/server/federation/policies/amrmproxy/LocalityMulticastAMRMProxyPolicy.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/main/java/org/apache/hadoop/yarn/server/federation/policies/amrmproxy/LocalityMulticastAMRMProxyPolicy.java @@ -361,15 +361,7 @@ public class LocalityMulticastAMRMProxyPolicy extends AbstractAMRMProxyPolicy { for (SubClusterId targetId : targetSCs) { // if the calculated request is non-empty add it to the answer if (containerNums.get(i) > 0) { - ResourceRequest out = - ResourceRequest.newInstance(originalResourceRequest.getPriority(), - originalResourceRequest.getResourceName(), - originalResourceRequest.getCapability(), - originalResourceRequest.getNumContainers(), - originalResourceRequest.getRelaxLocality(), - originalResourceRequest.getNodeLabelExpression(), - originalResourceRequest.getExecutionTypeRequest()); - out.setAllocationRequestId(allocationId); + ResourceRequest out = ResourceRequest.clone(originalResourceRequest); out.setNumContainers(containerNums.get(i)); if (ResourceRequest.isAnyLocation(out.getResourceName())) { allocationBookkeeper.addAnyRR(targetId, out); http://git-wip-us.apache.org/repos/asf/hadoop/blob/99948565/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/main/java/org/apache/hadoop/yarn/server/scheduler/ResourceRequestSet.java ---------------------------------------------------------------------- diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/main/java/org/apache/hadoop/yarn/server/scheduler/ResourceRequestSet.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/main/java/org/apache/hadoop/yarn/server/scheduler/ResourceRequestSet.java index b1e6b6e..cf24bbf 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/main/java/org/apache/hadoop/yarn/server/scheduler/ResourceRequestSet.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/main/java/org/apache/hadoop/yarn/server/scheduler/ResourceRequestSet.java @@ -165,7 +165,7 @@ public class ResourceRequestSet { // the same numContainers value Map<String, ResourceRequest> newAsks = new HashMap<>(); for (ResourceRequest rr : this.asks.values()) { - ResourceRequest clone = cloneResourceRequest(rr); + ResourceRequest clone = ResourceRequest.clone(rr); clone.setNumContainers(newValue); newAsks.put(clone.getResourceName(), clone); } @@ -176,22 +176,12 @@ public class ResourceRequestSet { throw new YarnException( "No ANY RR found in requestSet with numContainers=" + oldValue); } - ResourceRequest clone = cloneResourceRequest(rr); + ResourceRequest clone = ResourceRequest.clone(rr); clone.setNumContainers(newValue); this.asks.put(ResourceRequest.ANY, clone); } } - private ResourceRequest cloneResourceRequest(ResourceRequest rr) { - return ResourceRequest.newBuilder().priority(rr.getPriority()) - .resourceName(rr.getResourceName()).capability(rr.getCapability()) - .numContainers(rr.getNumContainers()) - .relaxLocality(rr.getRelaxLocality()) - .nodeLabelExpression(rr.getNodeLabelExpression()) - .executionTypeRequest(rr.getExecutionTypeRequest()) - .allocationRequestId(rr.getAllocationRequestId()).build(); - } - @Override public String toString() { StringBuilder builder = new StringBuilder(); http://git-wip-us.apache.org/repos/asf/hadoop/blob/99948565/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/main/java/org/apache/hadoop/yarn/server/utils/BuilderUtils.java ---------------------------------------------------------------------- diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/main/java/org/apache/hadoop/yarn/server/utils/BuilderUtils.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/main/java/org/apache/hadoop/yarn/server/utils/BuilderUtils.java index e06b55e..b6145c9 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/main/java/org/apache/hadoop/yarn/server/utils/BuilderUtils.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/main/java/org/apache/hadoop/yarn/server/utils/BuilderUtils.java @@ -375,18 +375,6 @@ public class BuilderUtils { return request; } - public static ResourceRequest newResourceRequest(ResourceRequest r) { - ResourceRequest request = recordFactory - .newRecordInstance(ResourceRequest.class); - request.setPriority(r.getPriority()); - request.setResourceName(r.getResourceName()); - request.setCapability(r.getCapability()); - request.setNumContainers(r.getNumContainers()); - request.setNodeLabelExpression(r.getNodeLabelExpression()); - request.setExecutionTypeRequest(r.getExecutionTypeRequest()); - return request; - } - public static ApplicationReport newApplicationReport( ApplicationId applicationId, ApplicationAttemptId applicationAttemptId, String user, String queue, String name, String host, int rpcPort, http://git-wip-us.apache.org/repos/asf/hadoop/blob/99948565/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/placement/LocalityAppPlacementAllocator.java ---------------------------------------------------------------------- diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/placement/LocalityAppPlacementAllocator.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/placement/LocalityAppPlacementAllocator.java index a0358b4..e1239a9 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/placement/LocalityAppPlacementAllocator.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/placement/LocalityAppPlacementAllocator.java @@ -251,14 +251,8 @@ public class LocalityAppPlacementAllocator <N extends SchedulerNode> } public ResourceRequest cloneResourceRequest(ResourceRequest request) { - ResourceRequest newRequest = ResourceRequest.newBuilder() - .priority(request.getPriority()) - .allocationRequestId(request.getAllocationRequestId()) - .resourceName(request.getResourceName()) - .capability(request.getCapability()) - .numContainers(1) - .relaxLocality(request.getRelaxLocality()) - .nodeLabelExpression(request.getNodeLabelExpression()).build(); + ResourceRequest newRequest = ResourceRequest.clone(request); + newRequest.setNumContainers(1); return newRequest; } http://git-wip-us.apache.org/repos/asf/hadoop/blob/99948565/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/Application.java ---------------------------------------------------------------------- diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/Application.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/Application.java index 7d1140d..9178009 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/Application.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/Application.java @@ -305,9 +305,8 @@ public class Application { // Note this down for next interaction with ResourceManager ask.remove(request); - ask.add( - org.apache.hadoop.yarn.server.utils.BuilderUtils.newResourceRequest( - request)); // clone to ensure the RM doesn't manipulate the same obj + // clone to ensure the RM doesn't manipulate the same obj + ask.add(ResourceRequest.clone(request)); if (LOG.isDebugEnabled()) { LOG.debug("addResourceRequest: applicationId=" + applicationId.getId() @@ -462,9 +461,8 @@ public class Application { // Note this for next interaction with ResourceManager ask.remove(request); - ask.add( - org.apache.hadoop.yarn.server.utils.BuilderUtils.newResourceRequest( - request)); // clone to ensure the RM doesn't manipulate the same obj + // clone to ensure the RM doesn't manipulate the same obj + ask.add(ResourceRequest.clone(request)); if(LOG.isDebugEnabled()) { LOG.debug("updateResourceRequest:" + " application=" + applicationId http://git-wip-us.apache.org/repos/asf/hadoop/blob/99948565/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/TestAppManager.java ---------------------------------------------------------------------- diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/TestAppManager.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/TestAppManager.java index e79ba08..6a6f9cf 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/TestAppManager.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/TestAppManager.java @@ -733,7 +733,7 @@ public class TestAppManager{ ResourceRequest.newInstance(Priority.newInstance(0), ResourceRequest.ANY, Resources.createResource(1025), 1, true); req.setNodeLabelExpression(RMNodeLabelsManager.NO_LABEL); - asContext.setAMContainerResourceRequest(cloneResourceRequest(req)); + asContext.setAMContainerResourceRequest(ResourceRequest.clone(req)); // getAMContainerResourceRequests uses a singleton list of // getAMContainerResourceRequest Assert.assertEquals(req, asContext.getAMContainerResourceRequest()); @@ -1099,25 +1099,11 @@ public class TestAppManager{ YarnConfiguration.DEFAULT_RM_SCHEDULER_MINIMUM_ALLOCATION_MB); } - private static ResourceRequest cloneResourceRequest(ResourceRequest req) { - return ResourceRequest.newInstance( - Priority.newInstance(req.getPriority().getPriority()), - new String(req.getResourceName()), - Resource.newInstance(req.getCapability().getMemorySize(), - req.getCapability().getVirtualCores()), - req.getNumContainers(), - req.getRelaxLocality(), - req.getNodeLabelExpression() != null - ? new String(req.getNodeLabelExpression()) : null, - ExecutionTypeRequest.newInstance( - req.getExecutionTypeRequest().getExecutionType())); - } - private static List<ResourceRequest> cloneResourceRequests( List<ResourceRequest> reqs) { List<ResourceRequest> cloneReqs = new ArrayList<>(); for (ResourceRequest req : reqs) { - cloneReqs.add(cloneResourceRequest(req)); + cloneReqs.add(ResourceRequest.clone(req)); } return cloneReqs; } --------------------------------------------------------------------- To unsubscribe, e-mail: common-commits-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-commits-h...@hadoop.apache.org
