Repository: incubator-slider Updated Branches: refs/heads/develop 9104caad8 -> 7a8f39f46
SLIDER-905 Container request fails when Slider requests container with node label and host constraints Project: http://git-wip-us.apache.org/repos/asf/incubator-slider/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-slider/commit/7a8f39f4 Tree: http://git-wip-us.apache.org/repos/asf/incubator-slider/tree/7a8f39f4 Diff: http://git-wip-us.apache.org/repos/asf/incubator-slider/diff/7a8f39f4 Branch: refs/heads/develop Commit: 7a8f39f46a140f560efb5ced8cf344f7b6b87327 Parents: 9104caa Author: Steve Loughran <ste...@apache.org> Authored: Tue Jun 16 13:17:51 2015 +0100 Committer: Steve Loughran <ste...@apache.org> Committed: Tue Jun 16 13:17:51 2015 +0100 ---------------------------------------------------------------------- .../appmaster/state/OutstandingRequest.java | 57 ++++++++++++++++++-- ...tRoleHistoryOutstandingRequestTracker.groovy | 51 ++++++++++++++++++ 2 files changed, 104 insertions(+), 4 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-slider/blob/7a8f39f4/slider-core/src/main/java/org/apache/slider/server/appmaster/state/OutstandingRequest.java ---------------------------------------------------------------------- diff --git a/slider-core/src/main/java/org/apache/slider/server/appmaster/state/OutstandingRequest.java b/slider-core/src/main/java/org/apache/slider/server/appmaster/state/OutstandingRequest.java index e674835..2d31ffd 100644 --- a/slider-core/src/main/java/org/apache/slider/server/appmaster/state/OutstandingRequest.java +++ b/slider-core/src/main/java/org/apache/slider/server/appmaster/state/OutstandingRequest.java @@ -23,6 +23,7 @@ import com.google.common.base.Preconditions; import org.apache.hadoop.yarn.api.records.Priority; import org.apache.hadoop.yarn.api.records.Resource; import org.apache.hadoop.yarn.client.api.AMRMClient; +import org.apache.hadoop.yarn.client.api.InvalidContainerRequestException; import org.apache.slider.server.appmaster.operations.CancelSingleRequest; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -61,6 +62,12 @@ public final class OutstandingRequest { public final String hostname; /** + * Optional label. This is cached as the request option (explicit-location + label) is forbidden, + * yet the label needs to be retained for escalation. + */ + public String label; + + /** * Requested time in millis. * <p> * Only valid after {@link #buildContainerRequest(Resource, RoleStatus, long, String)} @@ -178,12 +185,15 @@ public final class OutstandingRequest { Preconditions.checkArgument(resource != null, "null `resource` arg"); Preconditions.checkArgument(role != null, "null `role` arg"); + // cache label for escalation + label = labelExpression; requestedTimeMillis = time; escalationTimeoutMillis = time + role.getPlacementTimeoutSeconds() * 1000; String[] hosts; boolean relaxLocality; boolean strictPlacement = role.isStrictPlacement(); NodeInstance target = this.node; + String nodeLabels; if (target != null) { // placed request. Hostname is used in request @@ -197,6 +207,7 @@ public final class OutstandingRequest { // enable escalation for all but strict placements. escalated = false; mayEscalate = !strictPlacement; + nodeLabels = null; } else { // no hosts hosts = null; @@ -206,6 +217,7 @@ public final class OutstandingRequest { escalated = true; // and forbid it happening mayEscalate = false; + nodeLabels = labelExpression; } Priority pri = ContainerPriority.createPriority(roleId, !relaxLocality); priority = pri.getPriority(); @@ -214,8 +226,9 @@ public final class OutstandingRequest { null, pri, relaxLocality, - labelExpression); + nodeLabels); + validate(); return issuedRequest; } @@ -235,7 +248,7 @@ public final class OutstandingRequest { Priority pri = ContainerPriority.createPriority(roleId, true); String[] nodes; List<String> issuedRequestNodes = issuedRequest.getNodes(); - if (issuedRequestNodes != null) { + if (label == null && issuedRequestNodes != null) { nodes = issuedRequestNodes.toArray(new String[issuedRequestNodes.size()]); } else { nodes = null; @@ -247,8 +260,9 @@ public final class OutstandingRequest { null, pri, true, - issuedRequest.getNodeLabelExpression()); + label); issuedRequest = newRequest; + validate(); return issuedRequest; } @@ -342,7 +356,42 @@ public final class OutstandingRequest { * @return an operation that can be used to cancel the request */ public CancelSingleRequest createCancelOperation() { - Preconditions.checkState(issuedRequest!=null, "No issued request to cancel"); + Preconditions.checkState(issuedRequest != null, "No issued request to cancel"); return new CancelSingleRequest(issuedRequest); } + + + /** + * Valid if a node label expression specified on container request is valid or + * not. Mimics the logic in AMRMClientImpl, so can be used for preflight checking + * and in mock tests + * + */ + public void validate() throws InvalidContainerRequestException { + Preconditions.checkNotNull(issuedRequest, "request has not yet been built up"); + AMRMClient.ContainerRequest containerRequest = issuedRequest; + String exp = containerRequest.getNodeLabelExpression(); + + if (null == exp || exp.isEmpty()) { + return; + } + + // Don't support specifying >= 2 node labels in a node label expression now + if (exp.contains("&&") || exp.contains("||")) { + throw new InvalidContainerRequestException( + "Cannot specify more than two node labels" + + " in a single node label expression"); + } + + // Don't allow specify node label against ANY request + if ((containerRequest.getRacks() != null && + (!containerRequest.getRacks().isEmpty())) + || + (containerRequest.getNodes() != null && + (!containerRequest.getNodes().isEmpty()))) { + throw new InvalidContainerRequestException( + "Cannot specify node label with rack and node"); + } + } + } http://git-wip-us.apache.org/repos/asf/incubator-slider/blob/7a8f39f4/slider-core/src/test/groovy/org/apache/slider/server/appmaster/model/history/TestRoleHistoryOutstandingRequestTracker.groovy ---------------------------------------------------------------------- diff --git a/slider-core/src/test/groovy/org/apache/slider/server/appmaster/model/history/TestRoleHistoryOutstandingRequestTracker.groovy b/slider-core/src/test/groovy/org/apache/slider/server/appmaster/model/history/TestRoleHistoryOutstandingRequestTracker.groovy index 99e4190..653af84 100644 --- a/slider-core/src/test/groovy/org/apache/slider/server/appmaster/model/history/TestRoleHistoryOutstandingRequestTracker.groovy +++ b/slider-core/src/test/groovy/org/apache/slider/server/appmaster/model/history/TestRoleHistoryOutstandingRequestTracker.groovy @@ -208,6 +208,57 @@ class TestRoleHistoryOutstandingRequestTracker extends BaseMockAppStateTest { } /** + * If the placement does include a label, the initial request must + * <i>not</i> include it. + * The escalation request will contain the label, while + * leaving out the node list. + * retains the node list, but sets relaxLocality==true + * @throws Throwable + */ + @Test + public void testRequestLabelledPlacement() throws Throwable { + NodeInstance ni = new NodeInstance("host1", 0) + def req1 = tracker.newRequest(ni, 0) + def resource = factory.newResource() + resource.virtualCores = 1 + resource.memory = 48; + + def label = "workers" + // initial request + def yarnRequest = req1.buildContainerRequest(resource, role0Status, 0, label) + assert (yarnRequest.nodeLabelExpression == null) + assert (!yarnRequest.relaxLocality) + def yarnRequest2 = req1.escalate() + assert (yarnRequest2.nodeLabelExpression == label) + assert (yarnRequest2.relaxLocality) + } + + /** + * If the placement doesnt include a lablel, then the escalation request + * retains the node list, but sets relaxLocality==true + * @throws Throwable + */ + @Test + public void testRequestUnlabelledPlacement() throws Throwable { + NodeInstance ni = new NodeInstance("host1", 0) + def req1 = tracker.newRequest(ni, 0) + def resource = factory.newResource() + resource.virtualCores = 1 + resource.memory = 48; + + def label = null + // initial request + def yarnRequest = req1.buildContainerRequest(resource, role0Status, 0, label) + assert (yarnRequest.nodes != null) + assert (yarnRequest.nodeLabelExpression == null) + assert (!yarnRequest.relaxLocality) + def yarnRequest2 = req1.escalate() + assert (yarnRequest2.nodes != null) + assert (yarnRequest2.relaxLocality) + } + + + /** * Create a new request (always against host1) * @param r role status * @return (resource, oustanding-request)
