Repository: incubator-slider Updated Branches: refs/heads/develop b0ff607de -> 4083cffea
SLIDER-943 Container Escalation failing Project: http://git-wip-us.apache.org/repos/asf/incubator-slider/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-slider/commit/4083cffe Tree: http://git-wip-us.apache.org/repos/asf/incubator-slider/tree/4083cffe Diff: http://git-wip-us.apache.org/repos/asf/incubator-slider/diff/4083cffe Branch: refs/heads/develop Commit: 4083cffea0e8e4a69b97f820cd6b2b0a3dea039e Parents: b0ff607 Author: Steve Loughran <[email protected]> Authored: Mon Sep 28 14:33:40 2015 +0200 Committer: Steve Loughran <[email protected]> Committed: Mon Sep 28 14:33:40 2015 +0200 ---------------------------------------------------------------------- .../operations/AbstractRMOperation.java | 2 +- .../operations/ContainerRequestOperation.java | 17 +++++++- .../appmaster/state/OutstandingRequest.java | 32 +++++++++----- .../state/OutstandingRequestTracker.java | 3 +- .../server/appmaster/state/RoleHistory.java | 3 +- .../TestMockAppStateFlexDynamicRoles.groovy | 2 - .../model/mock/MockRMOperationHandler.groovy | 8 ++-- .../appmaster/model/mock/MockYarnEngine.groovy | 45 +++++++++++++++++--- 8 files changed, 82 insertions(+), 30 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-slider/blob/4083cffe/slider-core/src/main/java/org/apache/slider/server/appmaster/operations/AbstractRMOperation.java ---------------------------------------------------------------------- diff --git a/slider-core/src/main/java/org/apache/slider/server/appmaster/operations/AbstractRMOperation.java b/slider-core/src/main/java/org/apache/slider/server/appmaster/operations/AbstractRMOperation.java index da8d646..b5b27c5 100644 --- a/slider-core/src/main/java/org/apache/slider/server/appmaster/operations/AbstractRMOperation.java +++ b/slider-core/src/main/java/org/apache/slider/server/appmaster/operations/AbstractRMOperation.java @@ -23,7 +23,7 @@ public abstract class AbstractRMOperation { /** * Execute the operation * @param asyncRMClient client - * @param handler + * @param handler handler to perform the execution */ public abstract void execute(RMOperationHandlerActions handler); http://git-wip-us.apache.org/repos/asf/incubator-slider/blob/4083cffe/slider-core/src/main/java/org/apache/slider/server/appmaster/operations/ContainerRequestOperation.java ---------------------------------------------------------------------- diff --git a/slider-core/src/main/java/org/apache/slider/server/appmaster/operations/ContainerRequestOperation.java b/slider-core/src/main/java/org/apache/slider/server/appmaster/operations/ContainerRequestOperation.java index b8120ca..6685b2a 100644 --- a/slider-core/src/main/java/org/apache/slider/server/appmaster/operations/ContainerRequestOperation.java +++ b/slider-core/src/main/java/org/apache/slider/server/appmaster/operations/ContainerRequestOperation.java @@ -18,14 +18,20 @@ package org.apache.slider.server.appmaster.operations; +import com.google.common.base.Preconditions; +import org.apache.hadoop.yarn.api.records.Priority; import org.apache.hadoop.yarn.client.api.AMRMClient; import org.apache.slider.server.appmaster.state.ContainerPriority; +/** + * A container request operation + */ public class ContainerRequestOperation extends AbstractRMOperation { private final AMRMClient.ContainerRequest request; public ContainerRequestOperation(AMRMClient.ContainerRequest request) { + Preconditions.checkArgument(request != null, "Null container request"); this.request = request; } @@ -33,6 +39,14 @@ public class ContainerRequestOperation extends AbstractRMOperation { return request; } + public Priority getPriority() { + return request.getPriority(); + } + + public boolean getRelaxLocality() { + return request.getRelaxLocality(); + } + @Override public void execute(RMOperationHandlerActions handler) { handler.addContainerRequest(request); @@ -40,6 +54,7 @@ public class ContainerRequestOperation extends AbstractRMOperation { @Override public String toString() { - return "request container for " + ContainerPriority.toString(request.getPriority()); + return "request container for " + ContainerPriority.toString(getPriority()) + + " relaxLocality=" + getRelaxLocality(); } } http://git-wip-us.apache.org/repos/asf/incubator-slider/blob/4083cffe/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 2d31ffd..e269ccd 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 @@ -245,7 +245,9 @@ public final class OutstandingRequest { escalated = true; // this is now the priority - Priority pri = ContainerPriority.createPriority(roleId, true); + // it is tagged as unlocated because it needs to go into a different + // set of outstanding requests from the strict placements + Priority pri = ContainerPriority.createPriority(roleId, false); String[] nodes; List<String> issuedRequestNodes = issuedRequest.getNodes(); if (label == null && issuedRequestNodes != null) { @@ -254,14 +256,12 @@ public final class OutstandingRequest { nodes = null; } - AMRMClient.ContainerRequest newRequest = - new AMRMClient.ContainerRequest(issuedRequest.getCapability(), - nodes, - null, - pri, - true, - label); - issuedRequest = newRequest; + issuedRequest = new AMRMClient.ContainerRequest(issuedRequest.getCapability(), + nodes, + null, + pri, + true, + label); validate(); return issuedRequest; } @@ -380,7 +380,7 @@ public final class OutstandingRequest { if (exp.contains("&&") || exp.contains("||")) { throw new InvalidContainerRequestException( "Cannot specify more than two node labels" - + " in a single node label expression"); + + " in a single node label expression: " + this); } // Don't allow specify node label against ANY request @@ -390,8 +390,18 @@ public final class OutstandingRequest { (containerRequest.getNodes() != null && (!containerRequest.getNodes().isEmpty()))) { throw new InvalidContainerRequestException( - "Cannot specify node label with rack and node"); + "Cannot specify node label with rack and node: " + this); + } + + // relax priority + boolean hasLocation = ContainerPriority.hasLocation(priority); + if (containerRequest.getRelaxLocality() != !hasLocation) { + throw new InvalidContainerRequestException( + "relax location flag doesn't match container priority: " + + this); + } } + } http://git-wip-us.apache.org/repos/asf/incubator-slider/blob/4083cffe/slider-core/src/main/java/org/apache/slider/server/appmaster/state/OutstandingRequestTracker.java ---------------------------------------------------------------------- diff --git a/slider-core/src/main/java/org/apache/slider/server/appmaster/state/OutstandingRequestTracker.java b/slider-core/src/main/java/org/apache/slider/server/appmaster/state/OutstandingRequestTracker.java index 97d321c..a8787f0 100644 --- a/slider-core/src/main/java/org/apache/slider/server/appmaster/state/OutstandingRequestTracker.java +++ b/slider-core/src/main/java/org/apache/slider/server/appmaster/state/OutstandingRequestTracker.java @@ -340,8 +340,7 @@ public class OutstandingRequestTracker { // time to escalate CancelSingleRequest cancel = outstandingRequest.createCancelOperation(); operations.add(cancel); - AMRMClient.ContainerRequest escalated = - outstandingRequest.escalate(); + AMRMClient.ContainerRequest escalated = outstandingRequest.escalate(); operations.add(new ContainerRequestOperation(escalated)); } } http://git-wip-us.apache.org/repos/asf/incubator-slider/blob/4083cffe/slider-core/src/main/java/org/apache/slider/server/appmaster/state/RoleHistory.java ---------------------------------------------------------------------- diff --git a/slider-core/src/main/java/org/apache/slider/server/appmaster/state/RoleHistory.java b/slider-core/src/main/java/org/apache/slider/server/appmaster/state/RoleHistory.java index 1a7f537..78fdcc9 100644 --- a/slider-core/src/main/java/org/apache/slider/server/appmaster/state/RoleHistory.java +++ b/slider-core/src/main/java/org/apache/slider/server/appmaster/state/RoleHistory.java @@ -955,8 +955,7 @@ public class RoleHistory { * @return a (usually empty) list of cancel/request operations. */ public List<AbstractRMOperation> escalateOutstandingRequests() { - long now = now(); - return outstandingRequests.escalateOutstandingRequests(now); + return outstandingRequests.escalateOutstandingRequests(now()); } /** http://git-wip-us.apache.org/repos/asf/incubator-slider/blob/4083cffe/slider-core/src/test/groovy/org/apache/slider/server/appmaster/model/appstate/TestMockAppStateFlexDynamicRoles.groovy ---------------------------------------------------------------------- diff --git a/slider-core/src/test/groovy/org/apache/slider/server/appmaster/model/appstate/TestMockAppStateFlexDynamicRoles.groovy b/slider-core/src/test/groovy/org/apache/slider/server/appmaster/model/appstate/TestMockAppStateFlexDynamicRoles.groovy index 015c6a3..5d880b4 100644 --- a/slider-core/src/test/groovy/org/apache/slider/server/appmaster/model/appstate/TestMockAppStateFlexDynamicRoles.groovy +++ b/slider-core/src/test/groovy/org/apache/slider/server/appmaster/model/appstate/TestMockAppStateFlexDynamicRoles.groovy @@ -23,7 +23,6 @@ import groovy.util.logging.Slf4j import org.apache.hadoop.conf.Configuration import org.apache.hadoop.fs.Path import org.apache.slider.api.ResourceKeys -import org.apache.slider.common.SliderExitCodes import org.apache.slider.core.conf.ConfTreeOperations import org.apache.slider.core.exceptions.BadConfigException import org.apache.slider.server.appmaster.model.mock.BaseMockAppStateTest @@ -31,7 +30,6 @@ import org.apache.slider.server.appmaster.model.mock.MockAppState import org.apache.slider.server.appmaster.model.mock.MockRoles import org.apache.slider.server.appmaster.model.mock.MockYarnEngine import org.apache.slider.server.appmaster.state.MostRecentContainerReleaseSelector -import org.apache.slider.server.avro.LoadedRoleHistory import org.apache.slider.server.avro.RoleHistoryWriter import org.junit.Test http://git-wip-us.apache.org/repos/asf/incubator-slider/blob/4083cffe/slider-core/src/test/groovy/org/apache/slider/server/appmaster/model/mock/MockRMOperationHandler.groovy ---------------------------------------------------------------------- diff --git a/slider-core/src/test/groovy/org/apache/slider/server/appmaster/model/mock/MockRMOperationHandler.groovy b/slider-core/src/test/groovy/org/apache/slider/server/appmaster/model/mock/MockRMOperationHandler.groovy index a68ce02..c803b54 100644 --- a/slider-core/src/test/groovy/org/apache/slider/server/appmaster/model/mock/MockRMOperationHandler.groovy +++ b/slider-core/src/test/groovy/org/apache/slider/server/appmaster/model/mock/MockRMOperationHandler.groovy @@ -69,10 +69,10 @@ class MockRMOperationHandler extends RMOperationHandler { cancelled++; } } - -/** - * clear the history - */ + + /** + * clear the history + */ public void clear() { operations.clear() releases = 0; http://git-wip-us.apache.org/repos/asf/incubator-slider/blob/4083cffe/slider-core/src/test/groovy/org/apache/slider/server/appmaster/model/mock/MockYarnEngine.groovy ---------------------------------------------------------------------- diff --git a/slider-core/src/test/groovy/org/apache/slider/server/appmaster/model/mock/MockYarnEngine.groovy b/slider-core/src/test/groovy/org/apache/slider/server/appmaster/model/mock/MockYarnEngine.groovy index e3d509a..5860c6b 100644 --- a/slider-core/src/test/groovy/org/apache/slider/server/appmaster/model/mock/MockYarnEngine.groovy +++ b/slider-core/src/test/groovy/org/apache/slider/server/appmaster/model/mock/MockYarnEngine.groovy @@ -28,6 +28,7 @@ import org.apache.hadoop.yarn.client.api.AMRMClient import org.apache.slider.server.appmaster.operations.AbstractRMOperation import org.apache.slider.server.appmaster.operations.ContainerReleaseOperation import org.apache.slider.server.appmaster.operations.ContainerRequestOperation +import org.junit.Assert /** * This is an evolving engine to mock YARN operations @@ -64,13 +65,13 @@ class MockYarnEngine { allocator = new Allocator(cluster) } -/** - * Allocate a container from a request. The containerID will be - * unique, nodeId and other fields chosen internally with - * no such guarantees; resource and priority copied over - * @param request request - * @return container - */ + /** + * Allocate a container from a request. The containerID will be + * unique, nodeId and other fields chosen internally with + * no such guarantees; resource and priority copied over + * @param request request + * @return container + */ Container allocateContainer(AMRMClient.ContainerRequest request) { MockContainer allocated = allocator.allocate(request) if (allocated != null) { @@ -104,6 +105,7 @@ class MockYarnEngine { */ List<Container> execute(List<AbstractRMOperation> ops, List<ContainerId> released) { + validateRequests(ops) List<Container> allocation = []; ops.each { AbstractRMOperation op -> if (op instanceof ContainerReleaseOperation) { @@ -126,4 +128,33 @@ class MockYarnEngine { return allocation } + /** + * Try and mimic some of the logic of <code>AMRMClientImpl.checkLocalityRelaxationConflict</code> + * @param ops operations list + */ + void validateRequests(List<AbstractRMOperation> ops) { + // run through the requests and verify that they are all consistent. + List<ContainerRequestOperation> outstandingRequests = [] + for (AbstractRMOperation operation : ops) { + if (operation instanceof ContainerRequestOperation) { + ContainerRequestOperation containerRequest = (ContainerRequestOperation) operation + def amRequest = containerRequest.request + def priority = amRequest.priority + def relax = amRequest.relaxLocality + + outstandingRequests.each { req -> + + if (req.priority == priority && req.relaxLocality != relax) { + // mismatch in values + Assert.fail("operation $operation has incompatible request priority" + + " from outsanding request") + } + outstandingRequests << containerRequest + + } + + } + } + } + } \ No newline at end of file
