updated per Yang review

Author: DiannaHohensee

server/src/main/java/org/elasticsearch/cluster/routing/allocation/MoveDecision.java

@@ -92,8 +92,9 @@ public void writeTo(StreamOutput out) throws IOException {
      * Creates a move decision for the shard being able to remain on its current node, so the shard won't
      * be forced to move to another node.
      */
-    public static MoveDecision createMoveDecisionWithRemainYesDecision(Decision canRemainDecision) {
+    public static MoveDecision createRemainYesDecision(Decision canRemainDecision) {
         assert canRemainDecision.type() != Type.NO;
+        assert canRemainDecision.type() != Type.NOT_PREFERRED;
         if (canRemainDecision == Decision.YES) {
             return CACHED_STAY_DECISION;
         }
@@ -162,7 +163,7 @@ public boolean cannotRemainAndCanMove() {
      */
     public boolean cannotRemainAndCannotMove() {
         checkDecisionState();
-        return canRemainDecision.type() != Type.YES && canMoveDecision != AllocationDecision.YES;
+        return canRemain() == false && canMoveDecision != AllocationDecision.YES;
     }
 
     /**

server/src/main/java/org/elasticsearch/cluster/routing/allocation/allocator/BalancedShardsAllocator.java

@@ -927,7 +927,7 @@ private MoveDecision decideMove(ProjectIndex index, ShardRouting shardRouting, P
             RoutingNode routingNode = sourceNode.getRoutingNode();
             Decision canRemain = allocation.deciders().canRemain(shardRouting, routingNode, allocation);
             if (canRemain.type() != Decision.Type.NO && canRemain.type() != Decision.Type.NOT_PREFERRED) {
-                return MoveDecision.createMoveDecisionWithRemainYesDecision(canRemain);
+                return MoveDecision.createRemainYesDecision(canRemain);
             }
 
             // Check predicate to decide whether to assess movement options

server/src/test/java/org/elasticsearch/cluster/routing/allocation/MoveDecisionTests.java

@@ -29,12 +29,12 @@ public class MoveDecisionTests extends ESTestCase {
 
     public void testCachedDecisions() {
         // cached stay decision
-        MoveDecision stay1 = MoveDecision.createMoveDecisionWithRemainYesDecision(Decision.YES);
-        MoveDecision stay2 = MoveDecision.createMoveDecisionWithRemainYesDecision(Decision.YES);
+        MoveDecision stay1 = MoveDecision.createRemainYesDecision(Decision.YES);
+        MoveDecision stay2 = MoveDecision.createRemainYesDecision(Decision.YES);
         assertSame(stay1, stay2); // not in explain mode, so should use cached decision
 
-        stay1 = MoveDecision.createMoveDecisionWithRemainYesDecision(new Decision.Single(Type.YES, null, null, (Object[]) null));
-        stay2 = MoveDecision.createMoveDecisionWithRemainYesDecision(new Decision.Single(Type.YES, null, null, (Object[]) null));
+        stay1 = MoveDecision.createRemainYesDecision(new Decision.Single(Type.YES, null, null, (Object[]) null));
+        stay2 = MoveDecision.createRemainYesDecision(new Decision.Single(Type.YES, null, null, (Object[]) null));
         assertNotSame(stay1, stay2);
 
         // cached cannot move decision
@@ -57,14 +57,14 @@ public void testCachedDecisions() {
     }
 
     public void testStayDecision() {
-        MoveDecision stay = MoveDecision.createMoveDecisionWithRemainYesDecision(Decision.YES);
+        MoveDecision stay = MoveDecision.createRemainYesDecision(Decision.YES);
         assertTrue(stay.canRemain());
         assertFalse(stay.cannotRemainAndCanMove());
         assertTrue(stay.isDecisionTaken());
         assertNull(stay.getNodeDecisions());
         assertEquals(AllocationDecision.NO_ATTEMPT, stay.getAllocationDecision());
 
-        stay = MoveDecision.createMoveDecisionWithRemainYesDecision(Decision.YES);
+        stay = MoveDecision.createRemainYesDecision(Decision.YES);
         assertTrue(stay.canRemain());
         assertFalse(stay.cannotRemainAndCanMove());
         assertTrue(stay.isDecisionTaken());