Ignore numeric shard count if waiting for ALL (#31265)

Today, if GET /_cluster/health?wait_for_active_shards=all does not immediately succeed then it throws an exception due to an erroneous and unnecessary call to ActiveShardCount#enoughShardsActive(). This commit fixes this logic. Fixes #31151
elastic · Jun 13, 2018 · dc393c8 · dc393c8
1 parent 9df86dc
commit dc393c8
Show file tree

Hide file tree

Showing 2 changed files with 20 additions and 5 deletions.
diff --git a/...main/java/org/elasticsearch/action/admin/cluster/health/TransportClusterHealthAction.java b/...main/java/org/elasticsearch/action/admin/cluster/health/TransportClusterHealthAction.java
@@ -234,11 +234,11 @@ static int prepareResponse(final ClusterHealthRequest request, final ClusterHeal
             ActiveShardCount waitForActiveShards = request.waitForActiveShards();
             assert waitForActiveShards.equals(ActiveShardCount.DEFAULT) == false :
                 "waitForActiveShards must not be DEFAULT on the request object, instead it should be NONE";
-            if (waitForActiveShards.equals(ActiveShardCount.ALL)
-                    && response.getUnassignedShards() == 0
-                    && response.getInitializingShards() == 0) {
-                // if we are waiting for all shards to be active, then the num of unassigned and num of initializing shards must be 0
-                waitForCounter++;
+            if (waitForActiveShards.equals(ActiveShardCount.ALL)) {
+                if (response.getUnassignedShards() == 0 && response.getInitializingShards() == 0) {
+                    // if we are waiting for all shards to be active, then the num of unassigned and num of initializing shards must be 0
+                    waitForCounter++;
+                }
             } else if (waitForActiveShards.enoughShardsActive(response.getActiveShards())) {
                 // there are enough active shards to meet the requirements of the request
                 waitForCounter++;

diff --git a/...java/org/elasticsearch/action/admin/cluster/health/TransportClusterHealthActionTests.java b/...java/org/elasticsearch/action/admin/cluster/health/TransportClusterHealthActionTests.java
@@ -20,6 +20,7 @@
 package org.elasticsearch.action.admin.cluster.health;
 
 import org.elasticsearch.Version;
+import org.elasticsearch.action.support.ActiveShardCount;
 import org.elasticsearch.cluster.ClusterName;
 import org.elasticsearch.cluster.ClusterState;
 import org.elasticsearch.cluster.metadata.IndexMetaData;
@@ -61,6 +62,20 @@ public void testWaitForInitializingShards() throws Exception {
         assertThat(TransportClusterHealthAction.prepareResponse(request, response, clusterState, null), equalTo(0));
     }
 
+    public void testWaitForAllShards() {
+        final String[] indices = {"test"};
+        final ClusterHealthRequest request = new ClusterHealthRequest();
+        request.waitForActiveShards(ActiveShardCount.ALL);
+
+        ClusterState clusterState = randomClusterStateWithInitializingShards("test", 1);
+        ClusterHealthResponse response = new ClusterHealthResponse("", indices, clusterState);
+        assertThat(TransportClusterHealthAction.prepareResponse(request, response, clusterState, null), equalTo(0));
+
+        clusterState = ClusterState.builder(ClusterName.CLUSTER_NAME_SETTING.getDefault(Settings.EMPTY)).build();
+        response = new ClusterHealthResponse("", indices, clusterState);
+        assertThat(TransportClusterHealthAction.prepareResponse(request, response, clusterState, null), equalTo(1));
+    }
+
     ClusterState randomClusterStateWithInitializingShards(String index, final int initializingShards) {
         final IndexMetaData indexMetaData = IndexMetaData
             .builder(index)