Deciders should not by default collect yes'es (#52438)

AllocationDeciders would collect Yes decisions when not asking for debug
info. Changed to only include Yes decisions when debug is requested
(explain).

server/src/main/java/org/elasticsearch/cluster/routing/allocation/decider/AllocationDeciders.java

@@ -30,8 +30,6 @@ import org.elasticsearch.cluster.routing.allocation.RoutingAllocation;
 import java.util.Collection;
 import java.util.Collections;
 
-import static org.elasticsearch.cluster.routing.allocation.RoutingAllocation.DebugMode.EXCLUDE_YES_DECISIONS;
-
 /**
  * A composite {@link AllocationDecider} combining the "decision" of multiple
  * {@link AllocationDecider} implementations into a single allocation decision.
@@ -58,9 +56,8 @@ public class AllocationDeciders extends AllocationDecider {
                 } else {
                     ret.add(decision);
                 }
-            } else if (decision != Decision.ALWAYS
+            } else {
-                        && (allocation.getDebugMode() != EXCLUDE_YES_DECISIONS || decision.type() != Decision.Type.YES)) {
+                addDecision(ret, decision, allocation);
-                ret.add(decision);
             }
         }
         return ret;
@@ -86,11 +83,8 @@ public class AllocationDeciders extends AllocationDecider {
                 } else {
                     ret.add(decision);
                 }
-            } else if (decision != Decision.ALWAYS
+            } else {
-                        && (allocation.getDebugMode() != EXCLUDE_YES_DECISIONS || decision.type() != Decision.Type.YES)) {
+                addDecision(ret, decision, allocation);
-                // the assumption is that a decider that returns the static instance Decision#ALWAYS
-                // does not really implements canAllocate
-                ret.add(decision);
             }
         }
         return ret;
@@ -118,9 +112,8 @@ public class AllocationDeciders extends AllocationDecider {
                 } else {
                     ret.add(decision);
                 }
-            } else if (decision != Decision.ALWAYS
+            } else {
-                        && (allocation.getDebugMode() != EXCLUDE_YES_DECISIONS || decision.type() != Decision.Type.YES)) {
+                addDecision(ret, decision, allocation);
-                ret.add(decision);
             }
         }
         return ret;
@@ -138,9 +131,8 @@ public class AllocationDeciders extends AllocationDecider {
                 } else {
                     ret.add(decision);
                 }
-            } else if (decision != Decision.ALWAYS
+            } else {
-                        && (allocation.getDebugMode() != EXCLUDE_YES_DECISIONS || decision.type() != Decision.Type.YES)) {
+                addDecision(ret, decision, allocation);
-                ret.add(decision);
             }
         }
         return ret;
@@ -158,9 +150,8 @@ public class AllocationDeciders extends AllocationDecider {
                 } else {
                     ret.add(decision);
                 }
-            } else if (decision != Decision.ALWAYS
+            } else {
-                && (allocation.getDebugMode() != EXCLUDE_YES_DECISIONS || decision.type() != Decision.Type.YES)) {
+                addDecision(ret, decision, allocation);
-                ret.add(decision);
             }
         }
         return ret;
@@ -178,9 +169,8 @@ public class AllocationDeciders extends AllocationDecider {
                 } else {
                     ret.add(decision);
                 }
-            } else if (decision != Decision.ALWAYS
+            } else {
-                        && (allocation.getDebugMode() != EXCLUDE_YES_DECISIONS || decision.type() != Decision.Type.YES)) {
+                addDecision(ret, decision, allocation);
-                ret.add(decision);
             }
         }
         return ret;
@@ -198,9 +188,8 @@ public class AllocationDeciders extends AllocationDecider {
                 } else {
                     ret.add(decision);
                 }
-            } else if (decision != Decision.ALWAYS
+            } else {
-                        && (allocation.getDebugMode() != EXCLUDE_YES_DECISIONS || decision.type() != Decision.Type.YES)) {
+                addDecision(ret, decision, allocation);
-                ret.add(decision);
             }
         }
         return ret;
@@ -218,9 +207,8 @@ public class AllocationDeciders extends AllocationDecider {
                 } else {
                     ret.add(decision);
                 }
-            } else if (decision != Decision.ALWAYS
+            } else {
-                        && (allocation.getDebugMode() != EXCLUDE_YES_DECISIONS || decision.type() != Decision.Type.YES)) {
+                addDecision(ret, decision, allocation);
-                ret.add(decision);
             }
         }
         return ret;
@@ -247,11 +235,18 @@ public class AllocationDeciders extends AllocationDecider {
                 } else {
                     ret.add(decision);
                 }
-            } else if (decision != Decision.ALWAYS
+            } else {
-                        && (allocation.getDebugMode() != EXCLUDE_YES_DECISIONS || decision.type() != Decision.Type.YES)) {
+                addDecision(ret, decision, allocation);
-                ret.add(decision);
             }
         }
         return ret;
     }
+
+    private void addDecision(Decision.Multi ret, Decision decision, RoutingAllocation allocation) {
+        // We never add ALWAYS decisions and only add YES decisions when requested by debug mode (since Multi default is YES).
+        if (decision != Decision.ALWAYS
+            && (allocation.getDebugMode() == RoutingAllocation.DebugMode.ON || decision.type() != Decision.Type.YES)) {
+            ret.add(decision);
+        }
+    }
 }

server/src/test/java/org/elasticsearch/cluster/routing/allocation/decider/AllocationDecidersTests.java

@@ -0,0 +1,126 @@
+/*
+ * Licensed to Elasticsearch under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch 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.elasticsearch.cluster.routing.allocation.decider;
+
+import org.elasticsearch.Version;
+import org.elasticsearch.cluster.ClusterName;
+import org.elasticsearch.cluster.ClusterState;
+import org.elasticsearch.cluster.metadata.IndexMetaData;
+import org.elasticsearch.cluster.node.DiscoveryNode;
+import org.elasticsearch.cluster.routing.RecoverySource;
+import org.elasticsearch.cluster.routing.RoutingNode;
+import org.elasticsearch.cluster.routing.ShardRouting;
+import org.elasticsearch.cluster.routing.UnassignedInfo;
+import org.elasticsearch.cluster.routing.allocation.RoutingAllocation;
+import org.elasticsearch.index.shard.ShardId;
+import org.elasticsearch.test.ESTestCase;
+import org.hamcrest.Matcher;
+import org.hamcrest.Matchers;
+
+import java.util.Collection;
+import java.util.Collections;
+
+public class AllocationDecidersTests extends ESTestCase {
+
+    public void testDebugMode() {
+        verifyDebugMode(RoutingAllocation.DebugMode.ON, Matchers.hasSize(1));
+    }
+
+    public void testNoDebugMode() {
+        verifyDebugMode(RoutingAllocation.DebugMode.OFF, Matchers.empty());
+    }
+
+    public void testDebugExcludeYesMode() {
+        verifyDebugMode(RoutingAllocation.DebugMode.EXCLUDE_YES_DECISIONS, Matchers.empty());
+    }
+
+    private void verifyDebugMode(RoutingAllocation.DebugMode mode, Matcher<Collection<? extends Decision>> matcher) {
+        AllocationDeciders deciders = new AllocationDeciders(Collections.singleton(new AllocationDecider() {
+            @Override
+            public Decision canAllocate(ShardRouting shardRouting, RoutingNode node, RoutingAllocation allocation) {
+                return Decision.YES;
+            }
+
+            @Override
+            public Decision canRebalance(ShardRouting shardRouting, RoutingAllocation allocation) {
+                return Decision.YES;
+            }
+
+            @Override
+            public Decision canRemain(ShardRouting shardRouting, RoutingNode node, RoutingAllocation allocation) {
+                return Decision.YES;
+            }
+
+            @Override
+            public Decision canAllocate(ShardRouting shardRouting, RoutingAllocation allocation) {
+                return Decision.YES;
+            }
+
+            @Override
+            public Decision canAllocate(IndexMetaData indexMetaData, RoutingNode node, RoutingAllocation allocation) {
+                return Decision.YES;
+            }
+
+            @Override
+            public Decision canAllocate(RoutingNode node, RoutingAllocation allocation) {
+                return Decision.YES;
+            }
+
+            @Override
+            public Decision shouldAutoExpandToNode(IndexMetaData indexMetaData, DiscoveryNode node, RoutingAllocation allocation) {
+                return Decision.YES;
+            }
+
+            @Override
+            public Decision canRebalance(RoutingAllocation allocation) {
+                return Decision.YES;
+            }
+        }));
+
+        ClusterState clusterState = ClusterState.builder(new ClusterName("test")).build();
+        final RoutingAllocation allocation = new RoutingAllocation(deciders,
+            clusterState.getRoutingNodes(), clusterState, null, 0L);
+
+        allocation.setDebugMode(mode);
+        final UnassignedInfo unassignedInfo = new UnassignedInfo(UnassignedInfo.Reason.INDEX_CREATED, "_message");
+        final ShardRouting shardRouting = ShardRouting.newUnassigned(new ShardId("test", "testUUID", 0), true,
+            RecoverySource.ExistingStoreRecoverySource.INSTANCE, unassignedInfo);
+        IndexMetaData idx =
+            IndexMetaData.builder("idx").settings(settings(Version.CURRENT)).numberOfShards(1).numberOfReplicas(0).build();
+
+        RoutingNode routingNode = new RoutingNode("testNode", null);
+        verify(deciders.canAllocate(shardRouting, routingNode, allocation), matcher);
+        verify(deciders.canAllocate(idx, routingNode, allocation), matcher);
+        verify(deciders.canAllocate(shardRouting, allocation), matcher);
+        verify(deciders.canAllocate(routingNode, allocation), matcher);
+        verify(deciders.canRebalance(shardRouting, allocation), matcher);
+        verify(deciders.canRebalance(allocation), matcher);
+        verify(deciders.canRemain(shardRouting, routingNode, allocation), matcher);
+        verify(deciders.canForceAllocatePrimary(shardRouting, routingNode, allocation), matcher);
+        verify(deciders.shouldAutoExpandToNode(idx, null, allocation), matcher);
+    }
+
+    private void verify(Decision decision, Matcher<Collection<? extends Decision>> matcher) {
+        assertThat(decision.type(), Matchers.equalTo(Decision.Type.YES));
+        assertThat(decision, Matchers.instanceOf(Decision.Multi.class));
+        Decision.Multi multi = (Decision.Multi) decision;
+        assertThat(multi.getDecisions(), matcher);
+    }
+}