SLIDER-82 generation of onContainerAllocated() cancel requests moved into RoleHistory; this will allow it to also generate new allocations at the same time.
Project: http://git-wip-us.apache.org/repos/asf/incubator-slider/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-slider/commit/d2ea8537 Tree: http://git-wip-us.apache.org/repos/asf/incubator-slider/tree/d2ea8537 Diff: http://git-wip-us.apache.org/repos/asf/incubator-slider/diff/d2ea8537 Branch: refs/heads/feature/SLIDER-82-pass-3.1 Commit: d2ea8537a74b7b0ba09836d50d55d54dab2dc683 Parents: a3591eb Author: Steve Loughran <ste...@apache.org> Authored: Thu Nov 5 18:48:41 2015 +0000 Committer: Steve Loughran <ste...@apache.org> Committed: Thu Nov 5 18:48:41 2015 +0000 ---------------------------------------------------------------------- .../slider/server/appmaster/state/AppState.java | 26 ++++++++------------ .../appmaster/state/ContainerAllocation.java | 14 +++++++---- .../appmaster/state/OutstandingRequest.java | 2 +- .../state/OutstandingRequestTracker.java | 22 ++++++++++++++--- .../server/appmaster/state/RoleHistory.java | 5 +++- ...tRoleHistoryOutstandingRequestTracker.groovy | 13 ++++++++-- .../model/mock/BaseMockAppStateTest.groovy | 2 -- .../appmaster/model/mock/MockFactory.groovy | 2 +- 8 files changed, 54 insertions(+), 32 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-slider/blob/d2ea8537/slider-core/src/main/java/org/apache/slider/server/appmaster/state/AppState.java ---------------------------------------------------------------------- diff --git a/slider-core/src/main/java/org/apache/slider/server/appmaster/state/AppState.java b/slider-core/src/main/java/org/apache/slider/server/appmaster/state/AppState.java index 9e29af2..e47ef34 100644 --- a/slider-core/src/main/java/org/apache/slider/server/appmaster/state/AppState.java +++ b/slider-core/src/main/java/org/apache/slider/server/appmaster/state/AppState.java @@ -2087,13 +2087,13 @@ public class AppState { * a list of operations to perform * @param allocatedContainers the containers allocated * @param assignments the assignments of roles to containers - * @param releaseOperations any release operations + * @param operations any allocation or release operations */ public synchronized void onContainersAllocated(List<Container> allocatedContainers, List<ContainerAssignment> assignments, - List<AbstractRMOperation> releaseOperations) { + List<AbstractRMOperation> operations) { assignments.clear(); - releaseOperations.clear(); + operations.clear(); List<Container> ordered = roleHistory.prepareAllocationList(allocatedContainers); log.debug("onContainersAllocated(): Total containers allocated = {}", ordered.size()); for (Container container : ordered) { @@ -2118,23 +2118,14 @@ public class AppState { roleHistory.onContainerAllocated(container, desired, allocated); final ContainerAllocationOutcome outcome = allocation.outcome; - // cancel an allocation request which granted this, so as to avoid repeated - // requests - if (allocation.origin != null && allocation.origin.getIssuedRequest() != null) { - releaseOperations.add(allocation.origin.createCancelOperation()); - } else { - // there's a request, but no idea what to cancel. - // rather than try to recover from it inelegantly, (and cause more confusion), - // log the event, but otherwise continue - log.warn("Unexpected allocation of container " - + SliderUtils.containerToString(container)); - } + // add all requests to the operations list + operations.addAll(allocation.operations); //look for condition where we get more back than we asked if (allocated > desired) { log.info("Discarding surplus {} container {} on {}", roleName, cid, containerHostInfo); - releaseOperations.add(new ContainerReleaseOperation(cid)); + operations.add(new ContainerReleaseOperation(cid)); //register as a surplus node surplusNodes.add(cid); surplusContainers.inc(); @@ -2156,7 +2147,10 @@ public class AppState { assignments.add(new ContainerAssignment(container, role, outcome)); //add to the history - roleHistory.onContainerAssigned(container); + AbstractRMOperation request = roleHistory.onContainerAssigned(container); + if (request != null) { + operations.add(request); + } } } } http://git-wip-us.apache.org/repos/asf/incubator-slider/blob/d2ea8537/slider-core/src/main/java/org/apache/slider/server/appmaster/state/ContainerAllocation.java ---------------------------------------------------------------------- diff --git a/slider-core/src/main/java/org/apache/slider/server/appmaster/state/ContainerAllocation.java b/slider-core/src/main/java/org/apache/slider/server/appmaster/state/ContainerAllocation.java index 306ffb2..6bfe8ab 100644 --- a/slider-core/src/main/java/org/apache/slider/server/appmaster/state/ContainerAllocation.java +++ b/slider-core/src/main/java/org/apache/slider/server/appmaster/state/ContainerAllocation.java @@ -18,6 +18,11 @@ package org.apache.slider.server.appmaster.state; +import org.apache.slider.server.appmaster.operations.AbstractRMOperation; + +import java.util.ArrayList; +import java.util.List; + /** * This is just a tuple of the outcome of a container allocation */ @@ -35,11 +40,10 @@ public class ContainerAllocation { */ public OutstandingRequest origin; - public ContainerAllocation(ContainerAllocationOutcome outcome, - OutstandingRequest origin) { - this.outcome = outcome; - this.origin = origin; - } + /** + * A possibly empty list of requests to add to the follow-up actions + */ + public List<AbstractRMOperation> operations = new ArrayList<>(0); public ContainerAllocation() { } http://git-wip-us.apache.org/repos/asf/incubator-slider/blob/d2ea8537/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 602b48e..85bd259 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 @@ -375,7 +375,7 @@ public final class OutstandingRequest { * and in mock tests * */ - public void validate() throws InvalidContainerRequestException { + public void validate() throws InvalidContainerRequestException { Preconditions.checkNotNull(issuedRequest, "request has not yet been built up"); AMRMClient.ContainerRequest containerRequest = issuedRequest; String exp = containerRequest.getNodeLabelExpression(); http://git-wip-us.apache.org/repos/asf/incubator-slider/blob/d2ea8537/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 a8787f0..bf34d43 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 @@ -115,8 +115,14 @@ public class OutstandingRequestTracker { } /** - * Notification that a container has been allocated -drop it - * from the {@link #placedRequests} structure. + * Notification that a container has been allocated + * + * <ol> + * <li>drop it from the {@link #placedRequests} structure.</li> + * <li>generate the cancellation request</li> + * <li>for AA placement, any actions needed</li> + * </ol> + * * @param role role index * @param hostname hostname * @return the allocation outcome @@ -129,8 +135,7 @@ public class OutstandingRequestTracker { containerDetails); ContainerAllocation allocation = new ContainerAllocation(); ContainerAllocationOutcome outcome; - OutstandingRequest request = - placedRequests.remove(new OutstandingRequest(role, hostname)); + OutstandingRequest request = placedRequests.remove(new OutstandingRequest(role, hostname)); if (request != null) { //satisfied request log.debug("Found placed request for container: {}", request); @@ -154,6 +159,15 @@ public class OutstandingRequestTracker { outcome = ContainerAllocationOutcome.Unallocated; } } + if (request != null && request.getIssuedRequest() != null) { + allocation.operations.add(request.createCancelOperation()); + } else { + // there's a request, but no idea what to cancel. + // rather than try to recover from it inelegantly, (and cause more confusion), + // log the event, but otherwise continue + log.warn("Unexpected allocation of container " + SliderUtils.containerToString(container)); + } + allocation.origin = request; allocation.outcome = outcome; return allocation; http://git-wip-us.apache.org/repos/asf/incubator-slider/blob/d2ea8537/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 6880b69..a0aa3bc 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 @@ -749,6 +749,8 @@ public class RoleHistory { sortRecentNodeList(role); } } + // AA placement: now request a new node + return outcome; } @@ -756,9 +758,10 @@ public class RoleHistory { * A container has been assigned to a role instance on a node -update the data structures * @param container container */ - public void onContainerAssigned(Container container) { + public AbstractRMOperation onContainerAssigned(Container container) { NodeEntry nodeEntry = getOrCreateNodeEntry(container); nodeEntry.onStarting(); + return null; } /** http://git-wip-us.apache.org/repos/asf/incubator-slider/blob/d2ea8537/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 33cacd7..56b2c31 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 @@ -55,7 +55,11 @@ class TestRoleHistoryOutstandingRequestTracker extends BaseMockAppStateTest { tracker.newRequest(host1, 0) tracker.newRequest(host2, 0) tracker.newRequest(host1, 1) - assert tracker.onContainerAllocated(1, "host1", null).outcome == ContainerAllocationOutcome.Placed + + def allocation = tracker.onContainerAllocated(1, "host1", null) + assert allocation.outcome == ContainerAllocationOutcome.Placed + assert allocation.operations[0] instanceof CancelSingleRequest + assert !tracker.lookupPlacedRequest(1, "host1") assert tracker.lookupPlacedRequest(0, "host1") } @@ -83,8 +87,11 @@ class TestRoleHistoryOutstandingRequestTracker extends BaseMockAppStateTest { resource.virtualCores=1 resource.memory = 48; c1.setResource(resource) - ContainerAllocationOutcome outcome = tracker.onContainerAllocated(0, "host1", c1).outcome + + def allocation = tracker.onContainerAllocated(0, "host1", c1) + ContainerAllocationOutcome outcome = allocation.outcome assert outcome == ContainerAllocationOutcome.Unallocated + assert allocation.operations.empty assert tracker.listOpenRequests().size() == 1 } @@ -115,6 +122,8 @@ class TestRoleHistoryOutstandingRequestTracker extends BaseMockAppStateTest { def allocation = tracker.onContainerAllocated(0, nodeId.host, c1) assert tracker.listOpenRequests().size() == 0 + assert allocation.operations[0] instanceof CancelSingleRequest + assert allocation.outcome == ContainerAllocationOutcome.Open assert allocation.origin.is(req1) } http://git-wip-us.apache.org/repos/asf/incubator-slider/blob/d2ea8537/slider-core/src/test/groovy/org/apache/slider/server/appmaster/model/mock/BaseMockAppStateTest.groovy ---------------------------------------------------------------------- diff --git a/slider-core/src/test/groovy/org/apache/slider/server/appmaster/model/mock/BaseMockAppStateTest.groovy b/slider-core/src/test/groovy/org/apache/slider/server/appmaster/model/mock/BaseMockAppStateTest.groovy index babce9b..a065518 100644 --- a/slider-core/src/test/groovy/org/apache/slider/server/appmaster/model/mock/BaseMockAppStateTest.groovy +++ b/slider-core/src/test/groovy/org/apache/slider/server/appmaster/model/mock/BaseMockAppStateTest.groovy @@ -152,7 +152,6 @@ abstract class BaseMockAppStateTest extends SliderTestBase implements MockRoles return ri } - public NodeInstance nodeInstance(long age, int live0, int live1=0, int live2=0) { NodeInstance ni = new NodeInstance("age${age}-[${live0},${live1},$live2]", MockFactory.ROLE_COUNT) @@ -205,7 +204,6 @@ abstract class BaseMockAppStateTest extends SliderTestBase implements MockRoles return createStartAndStopNodes([]) } - /** * Create, Start and stop nodes * @param completionResults List filled in with the status on all completed nodes http://git-wip-us.apache.org/repos/asf/incubator-slider/blob/d2ea8537/slider-core/src/test/groovy/org/apache/slider/server/appmaster/model/mock/MockFactory.groovy ---------------------------------------------------------------------- diff --git a/slider-core/src/test/groovy/org/apache/slider/server/appmaster/model/mock/MockFactory.groovy b/slider-core/src/test/groovy/org/apache/slider/server/appmaster/model/mock/MockFactory.groovy index 6071ef0..25fdd8b 100644 --- a/slider-core/src/test/groovy/org/apache/slider/server/appmaster/model/mock/MockFactory.groovy +++ b/slider-core/src/test/groovy/org/apache/slider/server/appmaster/model/mock/MockFactory.groovy @@ -57,7 +57,7 @@ class MockFactory implements MockRoles { PlacementPolicy.STRICT, 2, 1) - // role 2: longer delay + // role 2: longer delay and anti-affinity public static final ProviderRole PROVIDER_ROLE2 = new ProviderRole( MockRoles.ROLE2, 2,