You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@slider.apache.org by st...@apache.org on 2015/11/07 00:29:51 UTC

[09/22] incubator-slider git commit: SLIDER-82 generation of onContainerAllocated() cancel requests moved into RoleHistory; this will allow it to also generate new allocations at the same time.

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 <st...@apache.org>
Authored: Thu Nov 5 18:48:41 2015 +0000
Committer: Steve Loughran <st...@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,