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 2014/10/31 22:24:43 UTC

[1/4] git commit: SLIDER-587: building test for dynamic role placement history

Repository: incubator-slider
Updated Branches:
  refs/heads/develop a9e81c0b6 -> f83ce7571


SLIDER-587: building test for dynamic role placement history


Project: http://git-wip-us.apache.org/repos/asf/incubator-slider/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-slider/commit/22b4b5e7
Tree: http://git-wip-us.apache.org/repos/asf/incubator-slider/tree/22b4b5e7
Diff: http://git-wip-us.apache.org/repos/asf/incubator-slider/diff/22b4b5e7

Branch: refs/heads/develop
Commit: 22b4b5e72aa458f4e7b5cf4741ee3304057d8ed2
Parents: 2ac8428
Author: Steve Loughran <st...@apache.org>
Authored: Fri Oct 31 16:54:37 2014 +0000
Committer: Steve Loughran <st...@apache.org>
Committed: Fri Oct 31 16:54:37 2014 +0000

----------------------------------------------------------------------
 .../slider/server/appmaster/state/AppState.java | 11 ++--
 .../server/appmaster/state/RoleHistory.java     |  6 +--
 .../server/appmaster/state/RoleInstance.java    |  5 ++
 .../TestMockAppStateContainerFailure.groovy     |  7 ++-
 .../TestMockAppStateDynamicRoles.groovy         | 54 ++++++++++++++++++--
 .../TestMockAppStateFlexDynamicRoles.groovy     |  5 +-
 .../TestMockAppStateRMOperations.groovy         |  3 +-
 .../TestMockAppStateRebuildOnAMRestart.groovy   | 10 ++--
 .../TestMockAppStateRolePlacement.groovy        |  4 +-
 .../model/mock/BaseMockAppStateTest.groovy      | 34 ++++++++++--
 .../appmaster/model/mock/MockAppState.groovy    | 14 +++++
 11 files changed, 130 insertions(+), 23 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-slider/blob/22b4b5e7/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 a69a60d..406086a 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
@@ -574,20 +574,21 @@ public class AppState {
    * @return a new provider role
    * @throws BadConfigException bad configuration
    */
+  @VisibleForTesting
   public ProviderRole createDynamicProviderRole(String name,
                                                 MapOperations component) throws
                                                         BadConfigException {
     String priOpt = component.getMandatoryOption(ResourceKeys.COMPONENT_PRIORITY);
-    int pri = SliderUtils.parseAndValidate("value of " + name + " " +
+    int priority = SliderUtils.parseAndValidate("value of " + name + " " +
         ResourceKeys.COMPONENT_PRIORITY,
         priOpt, 0, 1, -1);
-    log.info("Role {} assigned priority {}", name, pri);
+    log.info("Role {} assigned priority {}", name, priority);
     String placementOpt = component.getOption(
       ResourceKeys.COMPONENT_PLACEMENT_POLICY, "0");
     int placement = SliderUtils.parseAndValidate("value of " + name + " " +
         ResourceKeys.COMPONENT_PLACEMENT_POLICY,
         placementOpt, 0, 0, -1);
-    return new ProviderRole(name, pri, placement);
+    return new ProviderRole(name, priority, placement);
   }
 
 
@@ -694,11 +695,13 @@ public class AppState {
     for (String name : roleNames) {
       if (!roles.containsKey(name)) {
         // this is a new value
+        MapOperations component = resources.getComponent(name);
         log.info("Adding new role {}", name);
         ProviderRole dynamicRole = createDynamicProviderRole(name,
-                               resources.getComponent(name));
+            component);
         RoleStatus roleStatus = buildRole(dynamicRole);
         roleStatus.setDesired(getDesiredInstanceCount(resources, name));
+        log.info("New role {}", roleStatus);
         roleHistory.addNewProviderRole(dynamicRole);
       }
     }

http://git-wip-us.apache.org/repos/asf/incubator-slider/blob/22b4b5e7/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 2b35ad5..2b0ee18 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
@@ -430,9 +430,9 @@ public class RoleHistory {
   /**
    * Get the nodes for an ID -may be null
    * @param id role ID
-   * @return list
+   * @return potenially null list
    */
-  private LinkedList<NodeInstance> getNodesForRoleId(int id) {
+  private List<NodeInstance> getNodesForRoleId(int id) {
     return availableNodes.get(id);
   }
   
@@ -481,7 +481,7 @@ public class RoleHistory {
     
     List<NodeInstance> targets = getNodesForRoleId(roleKey);
     int cnt = targets == null ? 0 : targets.size();
-    log.debug("There're {} nodes to consider for {}", cnt, role.getName());
+    log.debug("There are {} node(s) to consider for {}", cnt, role.getName());
     while (targets != null && !targets.isEmpty() && nodeInstance == null) {
       NodeInstance head = targets.remove(0);
       if (head.getActiveRoleInstances(roleKey) == 0) {

http://git-wip-us.apache.org/repos/asf/incubator-slider/blob/22b4b5e7/slider-core/src/main/java/org/apache/slider/server/appmaster/state/RoleInstance.java
----------------------------------------------------------------------
diff --git a/slider-core/src/main/java/org/apache/slider/server/appmaster/state/RoleInstance.java b/slider-core/src/main/java/org/apache/slider/server/appmaster/state/RoleInstance.java
index c6d8f4c..c8ddc6f 100644
--- a/slider-core/src/main/java/org/apache/slider/server/appmaster/state/RoleInstance.java
+++ b/slider-core/src/main/java/org/apache/slider/server/appmaster/state/RoleInstance.java
@@ -51,8 +51,13 @@ public final class RoleInstance implements Cloneable {
    * already been targeted for termination
    */
   public boolean released;
+
+  /**
+   * Name of the role
+   */
   public String role;
   public int roleId;
+
   /**
    * state from {@link ClusterDescription}
    */

http://git-wip-us.apache.org/repos/asf/incubator-slider/blob/22b4b5e7/slider-core/src/test/groovy/org/apache/slider/server/appmaster/model/appstate/TestMockAppStateContainerFailure.groovy
----------------------------------------------------------------------
diff --git a/slider-core/src/test/groovy/org/apache/slider/server/appmaster/model/appstate/TestMockAppStateContainerFailure.groovy b/slider-core/src/test/groovy/org/apache/slider/server/appmaster/model/appstate/TestMockAppStateContainerFailure.groovy
index 9902155..6368a3d 100644
--- a/slider-core/src/test/groovy/org/apache/slider/server/appmaster/model/appstate/TestMockAppStateContainerFailure.groovy
+++ b/slider-core/src/test/groovy/org/apache/slider/server/appmaster/model/appstate/TestMockAppStateContainerFailure.groovy
@@ -29,7 +29,12 @@ import org.apache.slider.server.appmaster.actions.ResetFailureWindow
 import org.apache.slider.server.appmaster.model.mock.BaseMockAppStateTest
 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.*
+import org.apache.slider.server.appmaster.state.AppState
+import org.apache.slider.server.appmaster.state.NodeEntry
+import org.apache.slider.server.appmaster.state.NodeInstance
+import org.apache.slider.server.appmaster.state.RoleHistory
+import org.apache.slider.server.appmaster.state.RoleInstance
+import org.apache.slider.server.appmaster.state.RoleStatus
 import org.junit.Test
 
 /**

http://git-wip-us.apache.org/repos/asf/incubator-slider/blob/22b4b5e7/slider-core/src/test/groovy/org/apache/slider/server/appmaster/model/appstate/TestMockAppStateDynamicRoles.groovy
----------------------------------------------------------------------
diff --git a/slider-core/src/test/groovy/org/apache/slider/server/appmaster/model/appstate/TestMockAppStateDynamicRoles.groovy b/slider-core/src/test/groovy/org/apache/slider/server/appmaster/model/appstate/TestMockAppStateDynamicRoles.groovy
index 136e1ea..5ef639b 100644
--- a/slider-core/src/test/groovy/org/apache/slider/server/appmaster/model/appstate/TestMockAppStateDynamicRoles.groovy
+++ b/slider-core/src/test/groovy/org/apache/slider/server/appmaster/model/appstate/TestMockAppStateDynamicRoles.groovy
@@ -22,12 +22,13 @@ import groovy.transform.CompileStatic
 import groovy.util.logging.Slf4j
 import org.apache.hadoop.conf.Configuration
 import org.apache.slider.api.ResourceKeys
+import org.apache.slider.common.tools.SliderUtils
+import org.apache.slider.core.conf.ConfTreeOperations
 import org.apache.slider.server.appmaster.model.mock.BaseMockAppStateTest
-import org.apache.slider.server.appmaster.model.mock.MockRecordFactory
+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.operations.AbstractRMOperation
-import org.apache.slider.server.appmaster.state.AppState
 import org.apache.slider.server.appmaster.state.RoleInstance
 import org.apache.slider.server.appmaster.state.SimpleReleaseSelector
 import org.junit.Test
@@ -58,7 +59,7 @@ class TestMockAppStateDynamicRoles extends BaseMockAppStateTest
   @Override
   void initApp() {
     super.initApp()
-    appState = new AppState(new MockRecordFactory())
+    appState = new MockAppState()
     appState.setContainerLimits(RM_MAX_RAM, RM_MAX_CORES)
 
     def instance = factory.newInstanceDefinition(0,0,0)
@@ -90,5 +91,50 @@ class TestMockAppStateDynamicRoles extends BaseMockAppStateTest
     appState.getRoleHistory().dump();
     
   }
-  
+
+
+  @Test
+  public void testDynamicRoleHistory() throws Throwable {
+
+    // snapshot and patch existing spec
+    def resources = ConfTreeOperations.fromInstance(
+        appState.resourcesSnapshot.confTree)
+
+    def name = "dynamic"
+    def dynamicComp = resources.getOrAddComponent(name)
+    int priority = 8
+    int placement = 3
+    dynamicComp.put(ResourceKeys.COMPONENT_PRIORITY, "8")
+    dynamicComp.put(ResourceKeys.COMPONENT_INSTANCES, "1")
+    dynamicComp.put(ResourceKeys.COMPONENT_PLACEMENT_POLICY, "3")
+
+    def component = resources.getComponent(name)
+    String priOpt = component.getMandatoryOption(
+        ResourceKeys.COMPONENT_PRIORITY);
+    int parsedPriority = SliderUtils.parseAndValidate(
+        "value of " + name + " " + ResourceKeys.COMPONENT_PRIORITY,
+        priOpt, 0, 1, -1);
+    assert priority == parsedPriority
+
+    def newRole = appState.createDynamicProviderRole(name, component)
+    assert newRole.id == priority
+    
+    appState.updateResourceDefinitions(resources.confTree);
+    assert appState.roleMap[name] != null
+    def mappedRole = appState.roleMap[name]
+    assert mappedRole.id == priority
+
+    def priorityMap = appState.rolePriorityMap
+    assert priorityMap.size() == 4
+    def dynamicProviderRole
+    assert (dynamicProviderRole = priorityMap[priority]) != null
+    assert dynamicProviderRole.id == priority
+
+    // allocate the nodes
+    def allocations = createAndStartNodes()
+    assert allocations.size() == 1
+    RoleInstance ri = allocations[0]
+    assert ri.role == name
+    assert ri.roleId == priority
+  }
 }

http://git-wip-us.apache.org/repos/asf/incubator-slider/blob/22b4b5e7/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 5c9dce9..8308a13 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
@@ -26,6 +26,7 @@ import org.apache.slider.api.ResourceKeys
 import org.apache.slider.core.conf.ConfTreeOperations
 import org.apache.slider.core.exceptions.BadConfigException
 import org.apache.slider.server.appmaster.model.mock.BaseMockAppStateTest
+import org.apache.slider.server.appmaster.model.mock.MockAppState
 import org.apache.slider.server.appmaster.model.mock.MockRecordFactory
 import org.apache.slider.server.appmaster.model.mock.MockRoles
 import org.apache.slider.server.appmaster.model.mock.MockYarnEngine
@@ -60,7 +61,7 @@ class TestMockAppStateFlexDynamicRoles extends BaseMockAppStateTest
   @Override
   void initApp() {
     super.initApp()
-    appState = new AppState(new MockRecordFactory())
+    appState = new MockAppState()
     appState.setContainerLimits(RM_MAX_RAM, RM_MAX_CORES)
 
     def instance = factory.newInstanceDefinition(0, 0, 0)
@@ -174,7 +175,7 @@ class TestMockAppStateFlexDynamicRoles extends BaseMockAppStateTest
     //now reset the app state
     def historyWorkDir2 = new File("target/history" + testName + "-0002")
     def historyPath2 = new Path(historyWorkDir2.toURI())
-    appState = new AppState(new MockRecordFactory())
+    appState = new MockAppState()
     appState.setContainerLimits(RM_MAX_RAM, RM_MAX_CORES)
     appState.buildInstance(
         factory.newInstanceDefinition(0, 0, 0),

http://git-wip-us.apache.org/repos/asf/incubator-slider/blob/22b4b5e7/slider-core/src/test/groovy/org/apache/slider/server/appmaster/model/appstate/TestMockAppStateRMOperations.groovy
----------------------------------------------------------------------
diff --git a/slider-core/src/test/groovy/org/apache/slider/server/appmaster/model/appstate/TestMockAppStateRMOperations.groovy b/slider-core/src/test/groovy/org/apache/slider/server/appmaster/model/appstate/TestMockAppStateRMOperations.groovy
index e5ad4ae..ee5eead 100644
--- a/slider-core/src/test/groovy/org/apache/slider/server/appmaster/model/appstate/TestMockAppStateRMOperations.groovy
+++ b/slider-core/src/test/groovy/org/apache/slider/server/appmaster/model/appstate/TestMockAppStateRMOperations.groovy
@@ -31,7 +31,8 @@ import org.apache.slider.server.appmaster.operations.CancelRequestOperation
 import org.apache.slider.server.appmaster.operations.ContainerReleaseOperation
 import org.apache.slider.server.appmaster.operations.ContainerRequestOperation
 import org.apache.slider.server.appmaster.operations.RMOperationHandler
-import org.apache.slider.server.appmaster.state.*
+import org.apache.slider.server.appmaster.state.ContainerAssignment
+import org.apache.slider.server.appmaster.state.RoleInstance
 import org.junit.Test
 
 import static org.apache.slider.server.appmaster.state.ContainerPriority.buildPriority

http://git-wip-us.apache.org/repos/asf/incubator-slider/blob/22b4b5e7/slider-core/src/test/groovy/org/apache/slider/server/appmaster/model/appstate/TestMockAppStateRebuildOnAMRestart.groovy
----------------------------------------------------------------------
diff --git a/slider-core/src/test/groovy/org/apache/slider/server/appmaster/model/appstate/TestMockAppStateRebuildOnAMRestart.groovy b/slider-core/src/test/groovy/org/apache/slider/server/appmaster/model/appstate/TestMockAppStateRebuildOnAMRestart.groovy
index c2783f3..b48a683 100644
--- a/slider-core/src/test/groovy/org/apache/slider/server/appmaster/model/appstate/TestMockAppStateRebuildOnAMRestart.groovy
+++ b/slider-core/src/test/groovy/org/apache/slider/server/appmaster/model/appstate/TestMockAppStateRebuildOnAMRestart.groovy
@@ -24,10 +24,14 @@ import org.apache.hadoop.conf.Configuration
 import org.apache.hadoop.yarn.api.records.Container
 import org.apache.slider.api.StatusKeys
 import org.apache.slider.server.appmaster.model.mock.BaseMockAppStateTest
-import org.apache.slider.server.appmaster.model.mock.MockRecordFactory
+import org.apache.slider.server.appmaster.model.mock.MockAppState
 import org.apache.slider.server.appmaster.model.mock.MockRoles
 import org.apache.slider.server.appmaster.operations.AbstractRMOperation
-import org.apache.slider.server.appmaster.state.*
+import org.apache.slider.server.appmaster.state.NodeEntry
+import org.apache.slider.server.appmaster.state.NodeInstance
+import org.apache.slider.server.appmaster.state.NodeMap
+import org.apache.slider.server.appmaster.state.RoleInstance
+import org.apache.slider.server.appmaster.state.SimpleReleaseSelector
 import org.junit.Test
 
 /**
@@ -67,7 +71,7 @@ class TestMockAppStateRebuildOnAMRestart extends BaseMockAppStateTest
     NodeMap nodemap = appState.roleHistory.cloneNodemap()
 
     // now destroy the app state
-    appState = new AppState(new MockRecordFactory())
+    appState = new MockAppState()
 
     //and rebuild
     appState.buildInstance(

http://git-wip-us.apache.org/repos/asf/incubator-slider/blob/22b4b5e7/slider-core/src/test/groovy/org/apache/slider/server/appmaster/model/appstate/TestMockAppStateRolePlacement.groovy
----------------------------------------------------------------------
diff --git a/slider-core/src/test/groovy/org/apache/slider/server/appmaster/model/appstate/TestMockAppStateRolePlacement.groovy b/slider-core/src/test/groovy/org/apache/slider/server/appmaster/model/appstate/TestMockAppStateRolePlacement.groovy
index 17ebc31..6df8910 100644
--- a/slider-core/src/test/groovy/org/apache/slider/server/appmaster/model/appstate/TestMockAppStateRolePlacement.groovy
+++ b/slider-core/src/test/groovy/org/apache/slider/server/appmaster/model/appstate/TestMockAppStateRolePlacement.groovy
@@ -27,7 +27,9 @@ import org.apache.slider.server.appmaster.model.mock.MockRoles
 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.apache.slider.server.appmaster.state.*
+import org.apache.slider.server.appmaster.state.ContainerAssignment
+import org.apache.slider.server.appmaster.state.RoleHistoryUtils
+import org.apache.slider.server.appmaster.state.RoleInstance
 import org.junit.Test
 
 import static org.apache.slider.server.appmaster.state.ContainerPriority.extractRole

http://git-wip-us.apache.org/repos/asf/incubator-slider/blob/22b4b5e7/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 6c0f571..fa54256 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
@@ -43,7 +43,7 @@ abstract class BaseMockAppStateTest extends SliderTestBase implements MockRoles
   public static final int RM_MAX_RAM = 4096
   public static final int RM_MAX_CORES = 64
   MockFactory factory = new MockFactory()
-  AppState appState
+  MockAppState appState
   MockYarnEngine engine
   protected HadoopFS fs
   protected SliderFileSystem sliderFileSystem
@@ -86,7 +86,7 @@ abstract class BaseMockAppStateTest extends SliderTestBase implements MockRoles
     historyWorkDir = new File("target/history", historyDirName)
     historyPath = new Path(historyWorkDir.toURI())
     fs.delete(historyPath, true)
-    appState = new AppState(new MockRecordFactory())
+    appState = new MockAppState()
     appState.setContainerLimits(RM_MAX_RAM, RM_MAX_CORES)
     appState.buildInstance(
         buildInstanceDefinition(),
@@ -172,7 +172,7 @@ abstract class BaseMockAppStateTest extends SliderTestBase implements MockRoles
    */
   public ContainerStatus containerStatus(ContainerId cid) {
     ContainerStatus status = containerStatus(cid,
-                                             LauncherExitCodes.EXIT_CLIENT_INITIATED_SHUTDOWN)
+        LauncherExitCodes.EXIT_CLIENT_INITIATED_SHUTDOWN)
     return status
   }
 
@@ -203,6 +203,20 @@ abstract class BaseMockAppStateTest extends SliderTestBase implements MockRoles
       List<AppState.NodeCompletionResult> completionResults) {
     List<ContainerId> released = []
     List<RoleInstance> instances = createAndSubmitNodes(released)
+    processSubmissionOperations(instances, completionResults, released)
+    return instances
+  }
+
+  /**
+   * Process the start/stop operations from 
+   * @param instances
+   * @param completionResults
+   * @param released
+   */
+  public void processSubmissionOperations(
+      List<RoleInstance> instances,
+      List<AppState.NodeCompletionResult> completionResults,
+      List<ContainerId> released) {
     for (RoleInstance instance : instances) {
       assert appState.onNodeManagerContainerStarted(instance.containerId)
     }
@@ -212,7 +226,6 @@ abstract class BaseMockAppStateTest extends SliderTestBase implements MockRoles
         "released",
         0
     )
-    return instances
   }
 
   /**
@@ -256,6 +269,19 @@ abstract class BaseMockAppStateTest extends SliderTestBase implements MockRoles
   public List<RoleInstance> createAndSubmitNodes(
       List<ContainerId> released) {
     List<AbstractRMOperation> ops = appState.reviewRequestAndReleaseNodes()
+    return submitOperations(ops, released)
+  }
+
+  /**
+   * Process the RM operations and send <code>onContainersAllocated</code>
+   * events to the app state
+   * @param ops
+   * @param released
+   * @return
+   */
+  public List<RoleInstance> submitOperations(
+      List<AbstractRMOperation> ops,
+      List<ContainerId> released) {
     List<Container> allocatedContainers = engine.execute(ops, released)
     List<ContainerAssignment> assignments = [];
     List<AbstractRMOperation> operations = []

http://git-wip-us.apache.org/repos/asf/incubator-slider/blob/22b4b5e7/slider-core/src/test/groovy/org/apache/slider/server/appmaster/model/mock/MockAppState.groovy
----------------------------------------------------------------------
diff --git a/slider-core/src/test/groovy/org/apache/slider/server/appmaster/model/mock/MockAppState.groovy b/slider-core/src/test/groovy/org/apache/slider/server/appmaster/model/mock/MockAppState.groovy
index 25c957e..ad85b89 100644
--- a/slider-core/src/test/groovy/org/apache/slider/server/appmaster/model/mock/MockAppState.groovy
+++ b/slider-core/src/test/groovy/org/apache/slider/server/appmaster/model/mock/MockAppState.groovy
@@ -17,13 +17,27 @@
 
 package org.apache.slider.server.appmaster.model.mock
 
+import org.apache.slider.providers.ProviderRole
 import org.apache.slider.server.appmaster.state.AbstractRecordFactory
 import org.apache.slider.server.appmaster.state.AppState
 
+/**
+ * Extended app state that makes more things public
+ */
 class MockAppState extends AppState {
 
   public MockAppState(AbstractRecordFactory recordFactory) {
     super(recordFactory);
   }
 
+  /**
+   * Instance with a mock record factory
+   */
+  public MockAppState() {
+    super(new MockRecordFactory());
+  }
+
+  public Map<String, ProviderRole> getRoleMap() {
+    return super.roleMap;
+  }
 }


[4/4] git commit: Merge branch 'feature/SLIDER-587-dynamic_role_placement' into develop

Posted by st...@apache.org.
Merge branch 'feature/SLIDER-587-dynamic_role_placement' into develop


Project: http://git-wip-us.apache.org/repos/asf/incubator-slider/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-slider/commit/f83ce757
Tree: http://git-wip-us.apache.org/repos/asf/incubator-slider/tree/f83ce757
Diff: http://git-wip-us.apache.org/repos/asf/incubator-slider/diff/f83ce757

Branch: refs/heads/develop
Commit: f83ce7571aeafcf782d6462ef779479c1f08924c
Parents: a9e81c0 0b06d63
Author: Steve Loughran <st...@apache.org>
Authored: Fri Oct 31 21:24:32 2014 +0000
Committer: Steve Loughran <st...@apache.org>
Committed: Fri Oct 31 21:24:32 2014 +0000

----------------------------------------------------------------------
 .../slider/server/appmaster/state/AppState.java |  37 ++--
 .../server/appmaster/state/NodeEntry.java       |  12 +-
 .../server/appmaster/state/NodeInstance.java    |   4 +-
 .../server/appmaster/state/RoleHistory.java     |  52 +++--
 .../server/appmaster/state/RoleInstance.java    |   5 +
 .../TestMockAppStateContainerFailure.groovy     |   7 +-
 .../TestMockAppStateDynamicHistory.groovy       | 211 +++++++++++++++++++
 .../TestMockAppStateDynamicRoles.groovy         |  14 +-
 .../TestMockAppStateFlexDynamicRoles.groovy     |   5 +-
 .../TestMockAppStateRMOperations.groovy         |   3 +-
 .../TestMockAppStateRebuildOnAMRestart.groovy   |  10 +-
 .../TestMockAppStateRolePlacement.groovy        |   4 +-
 .../appmaster/model/mock/Allocator.groovy       |   8 +-
 .../model/mock/BaseMockAppStateTest.groovy      |  36 +++-
 .../appmaster/model/mock/MockAppState.groovy    |  25 +++
 15 files changed, 364 insertions(+), 69 deletions(-)
----------------------------------------------------------------------



[3/4] git commit: SLIDER-587 test for dynamic role replacement passing

Posted by st...@apache.org.
SLIDER-587 test for dynamic role replacement passing


Project: http://git-wip-us.apache.org/repos/asf/incubator-slider/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-slider/commit/0b06d631
Tree: http://git-wip-us.apache.org/repos/asf/incubator-slider/tree/0b06d631
Diff: http://git-wip-us.apache.org/repos/asf/incubator-slider/diff/0b06d631

Branch: refs/heads/develop
Commit: 0b06d631598097ea4d867252a2864d3ce374b768
Parents: 34f78ad
Author: Steve Loughran <st...@apache.org>
Authored: Fri Oct 31 21:24:24 2014 +0000
Committer: Steve Loughran <st...@apache.org>
Committed: Fri Oct 31 21:24:24 2014 +0000

----------------------------------------------------------------------
 .../slider/server/appmaster/state/AppState.java |  2 -
 .../server/appmaster/state/NodeEntry.java       | 12 ++---
 .../server/appmaster/state/NodeInstance.java    |  4 +-
 .../server/appmaster/state/RoleHistory.java     | 44 +++++++++------
 .../TestMockAppStateDynamicHistory.groovy       | 57 ++++++++++++++------
 .../appmaster/model/mock/MockAppState.groovy    | 11 ++++
 6 files changed, 86 insertions(+), 44 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-slider/blob/0b06d631/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 2b5d0ee..0848173 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
@@ -708,8 +708,6 @@ public class AppState {
         log.info("New role {}", roleStatus);
         roleHistory.addNewProviderRole(dynamicRole);
         newRoles.add(dynamicRole);
-      } else {
-        log.debug("known role: {}", name);
       }
     }
     return newRoles;

http://git-wip-us.apache.org/repos/asf/incubator-slider/blob/0b06d631/slider-core/src/main/java/org/apache/slider/server/appmaster/state/NodeEntry.java
----------------------------------------------------------------------
diff --git a/slider-core/src/main/java/org/apache/slider/server/appmaster/state/NodeEntry.java b/slider-core/src/main/java/org/apache/slider/server/appmaster/state/NodeEntry.java
index 83c590b..ebddaf9 100644
--- a/slider-core/src/main/java/org/apache/slider/server/appmaster/state/NodeEntry.java
+++ b/slider-core/src/main/java/org/apache/slider/server/appmaster/state/NodeEntry.java
@@ -37,10 +37,10 @@ package org.apache.slider.server.appmaster.state;
  */
 public class NodeEntry {
   
-  public final int index;
+  public final int rolePriority;
 
-  public NodeEntry(int index) {
-    this.index = index;
+  public NodeEntry(int rolePriority) {
+    this.rolePriority = rolePriority;
   }
 
   /**
@@ -132,8 +132,7 @@ public class NodeEntry {
   public synchronized boolean onStartFailed() {
     decStarting();
     ++startFailed;
-    ++failed;
-    return isAvailable();
+    return containerCompleted(false);
   }
   
   /**
@@ -211,7 +210,8 @@ public class NodeEntry {
   @Override
   public String toString() {
     final StringBuilder sb = new StringBuilder("NodeEntry{");
-    sb.append("requested=").append(requested);
+    sb.append("priority=").append(rolePriority);
+    sb.append(", requested=").append(requested);
     sb.append(", starting=").append(starting);
     sb.append(", live=").append(live);
     sb.append(", failed=").append(failed);

http://git-wip-us.apache.org/repos/asf/incubator-slider/blob/0b06d631/slider-core/src/main/java/org/apache/slider/server/appmaster/state/NodeInstance.java
----------------------------------------------------------------------
diff --git a/slider-core/src/main/java/org/apache/slider/server/appmaster/state/NodeInstance.java b/slider-core/src/main/java/org/apache/slider/server/appmaster/state/NodeInstance.java
index 1ba2282..bc79b71 100644
--- a/slider-core/src/main/java/org/apache/slider/server/appmaster/state/NodeInstance.java
+++ b/slider-core/src/main/java/org/apache/slider/server/appmaster/state/NodeInstance.java
@@ -52,7 +52,7 @@ public class NodeInstance {
    */
   public synchronized NodeEntry get(int role) {
     for (NodeEntry nodeEntry : nodeEntries) {
-      if (nodeEntry.index == role) {
+      if (nodeEntry.rolePriority == role) {
         return nodeEntry;
       }
     }
@@ -146,7 +146,7 @@ public class NodeInstance {
       new StringBuilder(toString());
     int i = 0;
     for (NodeEntry entry : nodeEntries) {
-      sb.append(String.format("\n  [%02d]  ", i++));
+      sb.append(String.format("\n  [%02d]  ", entry.rolePriority));
         sb.append(entry.toString());
     }
     return sb.toString();

http://git-wip-us.apache.org/repos/asf/incubator-slider/blob/0b06d631/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 33c3442..f1c0af5 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
@@ -63,8 +63,6 @@ public class RoleHistory {
   protected static final Logger log =
     LoggerFactory.getLogger(RoleHistory.class);
   private final List<ProviderRole> providerRoles;
-  private final Map<String, ProviderRole> providerRoleMap =
-    new HashMap<String, ProviderRole>();
   private long startTime;
   /**
    * Time when saved
@@ -97,6 +95,7 @@ public class RoleHistory {
    * ask with/without locality. Has other potential uses as well.
    */
   private Set<String> failedNodes = new HashSet<String>();
+  
   // dummy to be used in maps for faster lookup where we don't care about values
   private final Object DUMMY_VALUE = new Object(); 
 
@@ -104,9 +103,6 @@ public class RoleHistory {
                                                        BadConfigException {
     this.providerRoles = providerRoles;
     roleSize = providerRoles.size();
-    for (ProviderRole providerRole : providerRoles) {
-      providerRoleMap.put(providerRole.name, providerRole);
-    }
     reset();
   }
 
@@ -120,18 +116,24 @@ public class RoleHistory {
     resetAvailableNodeLists();
 
     outstandingRequests = new OutstandingRequestTracker();
+    
     Map<Integer, RoleStatus> roleStats = new HashMap<Integer, RoleStatus>();
-
-
     for (ProviderRole providerRole : providerRoles) {
-      addProviderRole(roleStats, providerRole);
+      checkProviderRole(roleStats, providerRole);
     }
   }
 
-  
-  private void addProviderRole(Map<Integer, RoleStatus> roleStats,
-                               ProviderRole providerRole)
-    throws ArrayIndexOutOfBoundsException, BadConfigException {
+  /**
+   * safety check: make sure the provider role is unique amongst
+   * the role stats...which is extended with the new role
+   * @param roleStats role stats
+   * @param providerRole role
+   * @throws ArrayIndexOutOfBoundsException
+   * @throws BadConfigException
+   */
+  private void checkProviderRole(Map<Integer, RoleStatus> roleStats,
+      ProviderRole providerRole)
+    throws BadConfigException {
     int index = providerRole.id;
     if (index < 0) {
       throw new BadConfigException("Provider " + providerRole
@@ -154,12 +156,12 @@ public class RoleHistory {
     throws BadConfigException {
     Map<Integer, RoleStatus> roleStats = new HashMap<Integer, RoleStatus>();
 
-
     for (ProviderRole role : providerRoles) {
       roleStats.put(role.id, new RoleStatus(role));
     }
 
-    addProviderRole(roleStats, providerRole);
+    checkProviderRole(roleStats, providerRole);
+    this.providerRoles.add(providerRole);
   }
 
   /**
@@ -432,7 +434,8 @@ public class RoleHistory {
    * @param id role ID
    * @return potenially null list
    */
-  private List<NodeInstance> getNodesForRoleId(int id) {
+  @VisibleForTesting
+  public List<NodeInstance> getNodesForRoleId(int id) {
     return availableNodes.get(id);
   }
   
@@ -610,6 +613,8 @@ public class RoleHistory {
   public synchronized boolean onContainerAllocated(Container container, int desiredCount, int actualCount) {
     int role = ContainerPriority.extractRole(container);
     String hostname = RoleHistoryUtils.hostnameOf(container);
+    LinkedList<NodeInstance> nodeInstances =
+        getOrCreateNodesForRoleId(role);
     boolean requestFound =
       outstandingRequests.onContainerAllocated(role, hostname);
     if (desiredCount <= actualCount) {
@@ -619,7 +624,7 @@ public class RoleHistory {
       if (!hosts.isEmpty()) {
         //add the list
         log.debug("Adding {} hosts for role {}", hosts.size(), role);
-        getOrCreateNodesForRoleId(role).addAll(hosts);
+        nodeInstances.addAll(hosts);
         sortAvailableNodeList(role);
       }
     }
@@ -734,6 +739,8 @@ public class RoleHistory {
                                                        boolean wasReleased,
                                                        boolean shortLived) {
     NodeEntry nodeEntry = getOrCreateNodeEntry(container);
+    log.debug("Finished container for node {}, released={}, shortlived={}",
+        nodeEntry.rolePriority, wasReleased, shortLived);
     boolean available;
     if (shortLived) {
       nodeEntry.onStartFailed();
@@ -781,13 +788,16 @@ public class RoleHistory {
       List<NodeInstance> instances =
         getOrCreateNodesForRoleId(role.id);
       log.info("  available: " + instances.size()
-               + " " + SliderUtils.joinWithInnerSeparator(", ", instances));
+               + " " + SliderUtils.joinWithInnerSeparator(" ", instances));
     }
 
     log.info("Nodes in Cluster: {}", getClusterSize());
     for (NodeInstance node : nodemap.values()) {
       log.info(node.toFullString());
     }
+
+    log.info("Failed nodes: {}",
+        SliderUtils.joinWithInnerSeparator(" ", failedNodes));
   }
 
   /**

http://git-wip-us.apache.org/repos/asf/incubator-slider/blob/0b06d631/slider-core/src/test/groovy/org/apache/slider/server/appmaster/model/appstate/TestMockAppStateDynamicHistory.groovy
----------------------------------------------------------------------
diff --git a/slider-core/src/test/groovy/org/apache/slider/server/appmaster/model/appstate/TestMockAppStateDynamicHistory.groovy b/slider-core/src/test/groovy/org/apache/slider/server/appmaster/model/appstate/TestMockAppStateDynamicHistory.groovy
index 9a9ad23..7d41012 100644
--- a/slider-core/src/test/groovy/org/apache/slider/server/appmaster/model/appstate/TestMockAppStateDynamicHistory.groovy
+++ b/slider-core/src/test/groovy/org/apache/slider/server/appmaster/model/appstate/TestMockAppStateDynamicHistory.groovy
@@ -24,7 +24,6 @@ import org.apache.hadoop.conf.Configuration
 import org.apache.hadoop.yarn.api.records.ContainerId
 import org.apache.slider.api.ResourceKeys
 import org.apache.slider.core.conf.ConfTreeOperations
-import org.apache.slider.providers.PlacementPolicy
 import org.apache.slider.providers.ProviderRole
 import org.apache.slider.server.appmaster.model.mock.BaseMockAppStateTest
 import org.apache.slider.server.appmaster.model.mock.MockAppState
@@ -84,7 +83,7 @@ class TestMockAppStateDynamicHistory extends BaseMockAppStateTest
   public void testDynamicRoleHistory() throws Throwable {
 
     def dynamic = "dynamicRole"
-    int priority_num_8 = 8
+    int role_priority_8 = 8
     int desired = 1
     int placementPolicy = 0
     // snapshot and patch existing spec
@@ -92,7 +91,7 @@ class TestMockAppStateDynamicHistory extends BaseMockAppStateTest
         appState.resourcesSnapshot.confTree)
     def opts = [
         (ResourceKeys.COMPONENT_INSTANCES): ""+desired,
-        (ResourceKeys.COMPONENT_PRIORITY) : "" +priority_num_8,
+        (ResourceKeys.COMPONENT_PRIORITY) : "" +role_priority_8,
         (ResourceKeys.COMPONENT_PLACEMENT_POLICY): "" + placementPolicy
     ]
 
@@ -109,21 +108,21 @@ class TestMockAppStateDynamicHistory extends BaseMockAppStateTest
     def snapshotDefinition = appState.resourcesSnapshot.getMandatoryComponent(
         dynamic)
     assert snapshotDefinition.getMandatoryOptionInt(
-        ResourceKeys.COMPONENT_PRIORITY) == priority_num_8
+        ResourceKeys.COMPONENT_PRIORITY) == role_priority_8
 
     // now look at the role map
     assert appState.roleMap[dynamic] != null
     def mappedRole = appState.roleMap[dynamic]
-    assert mappedRole.id == priority_num_8
+    assert mappedRole.id == role_priority_8
 
     def priorityMap = appState.rolePriorityMap
     assert priorityMap.size() == 4
     ProviderRole dynamicProviderRole
-    assert (dynamicProviderRole = priorityMap[priority_num_8]) != null
-    assert dynamicProviderRole.id == priority_num_8
+    assert (dynamicProviderRole = priorityMap[role_priority_8]) != null
+    assert dynamicProviderRole.id == role_priority_8
 
-    assert null != appState.roleStatusMap[priority_num_8]
-    def dynamicRoleStatus = appState.roleStatusMap[priority_num_8]
+    assert null != appState.roleStatusMap[role_priority_8]
+    def dynamicRoleStatus = appState.roleStatusMap[role_priority_8]
     assert dynamicRoleStatus.desired == desired
 
     
@@ -135,6 +134,9 @@ class TestMockAppStateDynamicHistory extends BaseMockAppStateTest
     assert targetNode == engine.allocator.nextIndex()
     def targetHostname = engine.cluster.nodeAt(targetNode).hostname
 
+    // clock is set to a small value
+    appState.time = 100000
+    
     // allocate the nodes
     def actions = appState.reviewRequestAndReleaseNodes()
     assert actions.size() == 1
@@ -150,31 +152,52 @@ class TestMockAppStateDynamicHistory extends BaseMockAppStateTest
     RoleInstance ri = allocations[0]
     
     assert ri.role == dynamic
-    assert ri.roleId == priority_num_8
+    assert ri.roleId == role_priority_8
     assert ri.host.host == targetHostname
 
     // now look at the role history
 
     def roleHistory = appState.roleHistory
-    def activeNodes = roleHistory.listActiveNodes(priority_num_8)
+    def activeNodes = roleHistory.listActiveNodes(role_priority_8)
     assert activeNodes.size() == 1
     NodeInstance activeNode = activeNodes[0]
+    assert activeNode.get(role_priority_8)
+    def entry8 = activeNode.get(role_priority_8)
+    assert entry8.active == 1
 
     assert activeNode.hostname == targetHostname
-    
-    // now trigger a termination event on that role
+
+    def activeNodeInstance = roleHistory.getOrCreateNodeInstance(ri.container)
+
+    assert activeNode == activeNodeInstance
+    def entry
+    assert (entry = activeNodeInstance.get(role_priority_8)) != null
+    assert entry.active
+    assert entry.live
 
 
+    // now trigger a termination event on that role
+    
+    // increment time for a long-lived failure event
+    appState.time = appState.time + 100000
+
+    log.debug("Triggering failure")
     def cid = ri.id
-    // failure
     AppState.NodeCompletionResult result = appState.onCompletedNode(
         containerStatus(cid, 1))
     assert result.roleInstance == ri
     assert result.containerFailed
+    
+    roleHistory.dump();
+    // values should have changed
+    assert entry.failed == 1
+    assert !entry.startFailed
+    assert !entry.active
+    assert !entry.live
+
 
-    def nodeForNewInstance = roleHistory.findNodeForNewInstance(
-        dynamicRoleStatus)
-    assert nodeForNewInstance
+    def nodesForRoleId = roleHistory.getNodesForRoleId(role_priority_8)
+    assert nodesForRoleId
     
     // make sure new nodes will default to a different host in the engine
     assert targetNode < engine.allocator.nextIndex()

http://git-wip-us.apache.org/repos/asf/incubator-slider/blob/0b06d631/slider-core/src/test/groovy/org/apache/slider/server/appmaster/model/mock/MockAppState.groovy
----------------------------------------------------------------------
diff --git a/slider-core/src/test/groovy/org/apache/slider/server/appmaster/model/mock/MockAppState.groovy b/slider-core/src/test/groovy/org/apache/slider/server/appmaster/model/mock/MockAppState.groovy
index ad85b89..e683587 100644
--- a/slider-core/src/test/groovy/org/apache/slider/server/appmaster/model/mock/MockAppState.groovy
+++ b/slider-core/src/test/groovy/org/apache/slider/server/appmaster/model/mock/MockAppState.groovy
@@ -30,6 +30,8 @@ class MockAppState extends AppState {
     super(recordFactory);
   }
 
+  long time = 0;
+  
   /**
    * Instance with a mock record factory
    */
@@ -40,4 +42,13 @@ class MockAppState extends AppState {
   public Map<String, ProviderRole> getRoleMap() {
     return super.roleMap;
   }
+
+  /**
+   * Current time. if the <code>time</code> field
+   * is set, that value is returned
+   * @return the current time.
+   */
+  protected long now() {
+    return time ?: System.currentTimeMillis();
+  }
 }


[2/4] git commit: SLIDER-587 test for role history on dynamic roles

Posted by st...@apache.org.
SLIDER-587 test for role history on dynamic roles


Project: http://git-wip-us.apache.org/repos/asf/incubator-slider/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-slider/commit/34f78ada
Tree: http://git-wip-us.apache.org/repos/asf/incubator-slider/tree/34f78ada
Diff: http://git-wip-us.apache.org/repos/asf/incubator-slider/diff/34f78ada

Branch: refs/heads/develop
Commit: 34f78ada7282a781dc6e440d9f4e4fd38d455f8d
Parents: 22b4b5e
Author: Steve Loughran <st...@apache.org>
Authored: Fri Oct 31 19:39:16 2014 +0000
Committer: Steve Loughran <st...@apache.org>
Committed: Fri Oct 31 19:39:16 2014 +0000

----------------------------------------------------------------------
 .../slider/server/appmaster/state/AppState.java |  30 +--
 .../server/appmaster/state/RoleHistory.java     |   4 -
 .../TestMockAppStateDynamicHistory.groovy       | 188 +++++++++++++++++++
 .../TestMockAppStateDynamicRoles.groovy         |  50 +----
 .../appmaster/model/mock/Allocator.groovy       |   8 +-
 .../model/mock/BaseMockAppStateTest.groovy      |   2 +-
 6 files changed, 214 insertions(+), 68 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-slider/blob/34f78ada/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 406086a..2b5d0ee 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
@@ -641,33 +641,36 @@ public class AppState {
   /**
    * The resource configuration is updated -review and update state.
    * @param resources updated resources specification
+   * @return a list of any dynamically added provider roles
+   * (purely for testing purposes)
    */
-  public synchronized void updateResourceDefinitions(ConfTree resources)
+  public synchronized List<ProviderRole> updateResourceDefinitions(ConfTree resources)
       throws BadConfigException, IOException {
     log.debug("Updating resources to {}", resources);
     
     instanceDefinition.setResources(resources);
     onInstanceDefinitionUpdated();
-    
-    
+ 
     //propagate the role table
-
     Map<String, Map<String, String>> updated = resources.components;
     getClusterStatus().roles = SliderUtils.deepClone(updated);
     getClusterStatus().updateTime = now();
-    buildRoleRequirementsFromResources();
+    return buildRoleRequirementsFromResources();
   }
 
   /**
    * build the role requirements from the cluster specification
+   * @return a list of any dynamically added provider roles
    */
-  private void buildRoleRequirementsFromResources() throws BadConfigException {
+  private List<ProviderRole> buildRoleRequirementsFromResources() throws BadConfigException {
 
+    List<ProviderRole> newRoles = new ArrayList<ProviderRole>(0);
+    
     //now update every role's desired count.
     //if there are no instance values, that role count goes to zero
 
     ConfTreeOperations resources =
-      instanceDefinition.getResourceOperations();
+        instanceDefinition.getResourceOperations();
 
     // Add all the existing roles
     for (RoleStatus roleStatus : getRoleStatusMap().values()) {
@@ -678,33 +681,38 @@ public class AppState {
       int currentDesired = roleStatus.getDesired();
       String role = roleStatus.getName();
       MapOperations comp =
-        resources.getComponent(role);
+          resources.getComponent(role);
       int desiredInstanceCount = getDesiredInstanceCount(resources, role);
       if (desiredInstanceCount == 0) {
         log.info("Role {} has 0 instances specified", role);
-      }  
+      }
       if (currentDesired != desiredInstanceCount) {
         log.info("Role {} flexed from {} to {}", role, currentDesired,
-                 desiredInstanceCount);
+            desiredInstanceCount);
         roleStatus.setDesired(desiredInstanceCount);
       }
     }
+
     //now the dynamic ones. Iterate through the the cluster spec and
     //add any role status entries not in the role status
     Set<String> roleNames = resources.getComponentNames();
     for (String name : roleNames) {
       if (!roles.containsKey(name)) {
         // this is a new value
-        MapOperations component = resources.getComponent(name);
         log.info("Adding new role {}", name);
+        MapOperations component = resources.getComponent(name);
         ProviderRole dynamicRole = createDynamicProviderRole(name,
             component);
         RoleStatus roleStatus = buildRole(dynamicRole);
         roleStatus.setDesired(getDesiredInstanceCount(resources, name));
         log.info("New role {}", roleStatus);
         roleHistory.addNewProviderRole(dynamicRole);
+        newRoles.add(dynamicRole);
+      } else {
+        log.debug("known role: {}", name);
       }
     }
+    return newRoles;
   }
 
   /**

http://git-wip-us.apache.org/repos/asf/incubator-slider/blob/34f78ada/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 2b0ee18..33c3442 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
@@ -462,10 +462,6 @@ public class RoleHistory {
     }
   }
 
-  public synchronized void onAMRestart() {
-    //TODO once AM restart is implemented and we know what to expect
-  }
-
   /**
    * Find a node for use
    * @param role role

http://git-wip-us.apache.org/repos/asf/incubator-slider/blob/34f78ada/slider-core/src/test/groovy/org/apache/slider/server/appmaster/model/appstate/TestMockAppStateDynamicHistory.groovy
----------------------------------------------------------------------
diff --git a/slider-core/src/test/groovy/org/apache/slider/server/appmaster/model/appstate/TestMockAppStateDynamicHistory.groovy b/slider-core/src/test/groovy/org/apache/slider/server/appmaster/model/appstate/TestMockAppStateDynamicHistory.groovy
new file mode 100644
index 0000000..9a9ad23
--- /dev/null
+++ b/slider-core/src/test/groovy/org/apache/slider/server/appmaster/model/appstate/TestMockAppStateDynamicHistory.groovy
@@ -0,0 +1,188 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.slider.server.appmaster.model.appstate
+
+import groovy.transform.CompileStatic
+import groovy.util.logging.Slf4j
+import org.apache.hadoop.conf.Configuration
+import org.apache.hadoop.yarn.api.records.ContainerId
+import org.apache.slider.api.ResourceKeys
+import org.apache.slider.core.conf.ConfTreeOperations
+import org.apache.slider.providers.PlacementPolicy
+import org.apache.slider.providers.ProviderRole
+import org.apache.slider.server.appmaster.model.mock.BaseMockAppStateTest
+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.operations.ContainerRequestOperation
+import org.apache.slider.server.appmaster.state.AppState
+import org.apache.slider.server.appmaster.state.NodeInstance
+import org.apache.slider.server.appmaster.state.RoleInstance
+import org.apache.slider.server.appmaster.state.SimpleReleaseSelector
+import org.junit.Test
+
+/**
+ * Test that if you have >1 role, the right roles are chosen for release.
+ */
+@CompileStatic
+@Slf4j
+class TestMockAppStateDynamicHistory extends BaseMockAppStateTest
+    implements MockRoles {
+
+  @Override
+  String getTestName() {
+    return "TestMockAppStateDynamicHistory"
+  }
+
+  /**
+   * Small cluster with multiple containers per node,
+   * to guarantee many container allocations on each node
+   * @return
+   */
+  @Override
+  MockYarnEngine createYarnEngine() {
+    return new MockYarnEngine(8, 1)
+  }
+
+  @Override
+  void initApp() {
+    super.initApp()
+    appState = new MockAppState()
+    appState.setContainerLimits(RM_MAX_RAM, RM_MAX_CORES)
+
+    def instance = factory.newInstanceDefinition(0,0,0)
+
+    appState.buildInstance(
+        instance,
+        new Configuration(),
+        new Configuration(false),
+        factory.ROLES,
+        fs,
+        historyPath,
+        null,
+        null, new SimpleReleaseSelector())
+  }
+
+
+  @Test
+  public void testDynamicRoleHistory() throws Throwable {
+
+    def dynamic = "dynamicRole"
+    int priority_num_8 = 8
+    int desired = 1
+    int placementPolicy = 0
+    // snapshot and patch existing spec
+    def resources = ConfTreeOperations.fromInstance(
+        appState.resourcesSnapshot.confTree)
+    def opts = [
+        (ResourceKeys.COMPONENT_INSTANCES): ""+desired,
+        (ResourceKeys.COMPONENT_PRIORITY) : "" +priority_num_8,
+        (ResourceKeys.COMPONENT_PLACEMENT_POLICY): "" + placementPolicy
+    ]
+
+    resources.components[dynamic] = opts
+
+
+    // write the definitions
+    def updates = appState.updateResourceDefinitions(resources.confTree);
+    assert updates.size() == 1
+    def updatedRole = updates[0]
+    assert updatedRole.placementPolicy == placementPolicy
+
+    // verify the new role was persisted
+    def snapshotDefinition = appState.resourcesSnapshot.getMandatoryComponent(
+        dynamic)
+    assert snapshotDefinition.getMandatoryOptionInt(
+        ResourceKeys.COMPONENT_PRIORITY) == priority_num_8
+
+    // now look at the role map
+    assert appState.roleMap[dynamic] != null
+    def mappedRole = appState.roleMap[dynamic]
+    assert mappedRole.id == priority_num_8
+
+    def priorityMap = appState.rolePriorityMap
+    assert priorityMap.size() == 4
+    ProviderRole dynamicProviderRole
+    assert (dynamicProviderRole = priorityMap[priority_num_8]) != null
+    assert dynamicProviderRole.id == priority_num_8
+
+    assert null != appState.roleStatusMap[priority_num_8]
+    def dynamicRoleStatus = appState.roleStatusMap[priority_num_8]
+    assert dynamicRoleStatus.desired == desired
+
+    
+    // before allocating the nodes, fill up the capacity of some of the
+    // hosts
+    engine.allocator.nextIndex()
+
+    def targetNode = 2
+    assert targetNode == engine.allocator.nextIndex()
+    def targetHostname = engine.cluster.nodeAt(targetNode).hostname
+
+    // allocate the nodes
+    def actions = appState.reviewRequestAndReleaseNodes()
+    assert actions.size() == 1
+    def action0 = (ContainerRequestOperation)actions[0]
+
+    def request = action0.request
+    assert !request.nodes
+
+    List<ContainerId> released = []
+    List<RoleInstance> allocations = submitOperations(actions, released)
+    processSubmissionOperations(allocations, [], released)
+    assert allocations.size() == 1
+    RoleInstance ri = allocations[0]
+    
+    assert ri.role == dynamic
+    assert ri.roleId == priority_num_8
+    assert ri.host.host == targetHostname
+
+    // now look at the role history
+
+    def roleHistory = appState.roleHistory
+    def activeNodes = roleHistory.listActiveNodes(priority_num_8)
+    assert activeNodes.size() == 1
+    NodeInstance activeNode = activeNodes[0]
+
+    assert activeNode.hostname == targetHostname
+    
+    // now trigger a termination event on that role
+
+
+    def cid = ri.id
+    // failure
+    AppState.NodeCompletionResult result = appState.onCompletedNode(
+        containerStatus(cid, 1))
+    assert result.roleInstance == ri
+    assert result.containerFailed
+
+    def nodeForNewInstance = roleHistory.findNodeForNewInstance(
+        dynamicRoleStatus)
+    assert nodeForNewInstance
+    
+    // make sure new nodes will default to a different host in the engine
+    assert targetNode < engine.allocator.nextIndex()
+
+    actions = appState.reviewRequestAndReleaseNodes()
+    assert actions.size() == 1
+    def action1 = (ContainerRequestOperation) actions[0]
+    def request1 = action1.request
+    assert request1.nodes
+  }
+}

http://git-wip-us.apache.org/repos/asf/incubator-slider/blob/34f78ada/slider-core/src/test/groovy/org/apache/slider/server/appmaster/model/appstate/TestMockAppStateDynamicRoles.groovy
----------------------------------------------------------------------
diff --git a/slider-core/src/test/groovy/org/apache/slider/server/appmaster/model/appstate/TestMockAppStateDynamicRoles.groovy b/slider-core/src/test/groovy/org/apache/slider/server/appmaster/model/appstate/TestMockAppStateDynamicRoles.groovy
index 5ef639b..902752c 100644
--- a/slider-core/src/test/groovy/org/apache/slider/server/appmaster/model/appstate/TestMockAppStateDynamicRoles.groovy
+++ b/slider-core/src/test/groovy/org/apache/slider/server/appmaster/model/appstate/TestMockAppStateDynamicRoles.groovy
@@ -28,7 +28,6 @@ import org.apache.slider.server.appmaster.model.mock.BaseMockAppStateTest
 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.operations.AbstractRMOperation
 import org.apache.slider.server.appmaster.state.RoleInstance
 import org.apache.slider.server.appmaster.state.SimpleReleaseSelector
 import org.junit.Test
@@ -86,55 +85,10 @@ class TestMockAppStateDynamicRoles extends BaseMockAppStateTest
   @Test
   public void testAllocateReleaseRealloc() throws Throwable {
 
-    List<RoleInstance> instances = createAndStartNodes()
-    List<AbstractRMOperation> ops = appState.reviewRequestAndReleaseNodes()
+    createAndStartNodes()
+    appState.reviewRequestAndReleaseNodes()
     appState.getRoleHistory().dump();
     
   }
 
-
-  @Test
-  public void testDynamicRoleHistory() throws Throwable {
-
-    // snapshot and patch existing spec
-    def resources = ConfTreeOperations.fromInstance(
-        appState.resourcesSnapshot.confTree)
-
-    def name = "dynamic"
-    def dynamicComp = resources.getOrAddComponent(name)
-    int priority = 8
-    int placement = 3
-    dynamicComp.put(ResourceKeys.COMPONENT_PRIORITY, "8")
-    dynamicComp.put(ResourceKeys.COMPONENT_INSTANCES, "1")
-    dynamicComp.put(ResourceKeys.COMPONENT_PLACEMENT_POLICY, "3")
-
-    def component = resources.getComponent(name)
-    String priOpt = component.getMandatoryOption(
-        ResourceKeys.COMPONENT_PRIORITY);
-    int parsedPriority = SliderUtils.parseAndValidate(
-        "value of " + name + " " + ResourceKeys.COMPONENT_PRIORITY,
-        priOpt, 0, 1, -1);
-    assert priority == parsedPriority
-
-    def newRole = appState.createDynamicProviderRole(name, component)
-    assert newRole.id == priority
-    
-    appState.updateResourceDefinitions(resources.confTree);
-    assert appState.roleMap[name] != null
-    def mappedRole = appState.roleMap[name]
-    assert mappedRole.id == priority
-
-    def priorityMap = appState.rolePriorityMap
-    assert priorityMap.size() == 4
-    def dynamicProviderRole
-    assert (dynamicProviderRole = priorityMap[priority]) != null
-    assert dynamicProviderRole.id == priority
-
-    // allocate the nodes
-    def allocations = createAndStartNodes()
-    assert allocations.size() == 1
-    RoleInstance ri = allocations[0]
-    assert ri.role == name
-    assert ri.roleId == priority
-  }
 }

http://git-wip-us.apache.org/repos/asf/incubator-slider/blob/34f78ada/slider-core/src/test/groovy/org/apache/slider/server/appmaster/model/mock/Allocator.groovy
----------------------------------------------------------------------
diff --git a/slider-core/src/test/groovy/org/apache/slider/server/appmaster/model/mock/Allocator.groovy b/slider-core/src/test/groovy/org/apache/slider/server/appmaster/model/mock/Allocator.groovy
index 639c632..a027098 100644
--- a/slider-core/src/test/groovy/org/apache/slider/server/appmaster/model/mock/Allocator.groovy
+++ b/slider-core/src/test/groovy/org/apache/slider/server/appmaster/model/mock/Allocator.groovy
@@ -51,11 +51,11 @@ class Allocator {
   MockContainer allocate(AMRMClient.ContainerRequest request) {
     MockYarnCluster.MockYarnClusterNode node = null
     MockYarnCluster.MockYarnClusterContainer allocated = null
-    if (request.nodes != null) {
+    if (request.nodes) {
       for (String host : request.nodes) {
         node = cluster.lookup(host)
         allocated = node.allocate()
-        if (allocated != null) {
+        if (allocated) {
           break
         }
       }
@@ -64,7 +64,7 @@ class Allocator {
     if (allocated) {
       return createContainerRecord(request, allocated, node)
     } else {
-      if (request.relaxLocality || request.nodes.isEmpty()) {
+      if (request.relaxLocality || request.nodes.empty) {
         // fallback to anywhere
         return allocateRandom(request)
       } else {
@@ -117,7 +117,7 @@ class Allocator {
     return container;
   }
 
-  private int nextIndex() {
+  public int nextIndex() {
     rollingIndex = (rollingIndex + 1) % cluster.clusterSize;
     return rollingIndex;
   }

http://git-wip-us.apache.org/repos/asf/incubator-slider/blob/34f78ada/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 fa54256..c48d7fa 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
@@ -136,7 +136,7 @@ abstract class BaseMockAppStateTest extends SliderTestBase implements MockRoles
     Container target = assigned.container
     RoleInstance ri = new RoleInstance(target)
     ri.roleId = assigned.role.priority
-    ri.role = assigned.role
+    ri.role = assigned.role.name
     return ri
   }