Adds wait_for_no_initializing_shards to cluster health API #27489

dnhatn · 2017-11-22T03:47:10Z

This adds a new option to the cluster health request allowing to wait until there is no initializing shards.

This adds a new option to the cluster health request allowing to wait until there is no initializing shards. Closes elastic#25623

ywelsch

Left some minor comments and asks, general change looks good.

ywelsch · 2017-11-22T16:12:29Z

core/src/test/java/org/elasticsearch/action/admin/cluster/health/ClusterHealthRequestTests.java

+    ClusterHealthRequest randomRequest() {
+        ClusterHealthRequest request = new ClusterHealthRequest();
+        request.waitForStatus(randomFrom(ClusterHealthStatus.values()));
+        request.waitForNodes(randomAlphaOfLengthBetween(5, 10));


wondering why this works. this should be a number or >=5, <7, ...?

We did not validate the request. I've updated the nodes expression.

ywelsch · 2017-11-22T16:15:39Z

...st/java/org/elasticsearch/action/admin/cluster/health/TransportClusterHealthActionTests.java

+            .build();
+        final ShardId shardId = new ShardId(new Index("index", "uuid"), 0);
+        final IndexRoutingTable.Builder routingTable = new IndexRoutingTable.Builder(indexMetaData.getIndex())
+            .addShard(TestShardRouting.newShardRouting(shardId, "node-0", true, ShardRoutingState.STARTED));


primary always started? what if that one is initializing? This is not well randomized.... it just randomizes in the number of initializing replicas...

I agreed. I've increased the randomness.

ywelsch · 2017-11-22T16:19:34Z

rest-api-spec/src/main/resources/rest-api-spec/test/cluster.health/10_basic.yml

+        body:
+          settings:
+            index:
+              number_of_replicas: 0


creating an index automatically waits for primary to be allocated (wait_for_active_shards is 1 by default). This means that the health check later is fully redundant, it will always succeed. This test does not really test anything here.

how about changing it so that the index is created with wait_for_active_shards : 0? Then this health check will actually wait for the primaries to be allocated and the output should guarantee that number of initializing shards is 0?

Thanks @ywelsch, your suggestion really tests the new parameter :). I pushed 04e61fb

ywelsch · 2017-11-22T16:21:08Z

...rc/main/java/org/elasticsearch/action/admin/cluster/health/TransportClusterHealthAction.java

-        if (request.indices() == null || request.indices().length == 0) { // check that they actually exists in the meta data
-            waitFor--;
+
+        if (request.waitForNoInitializingShards()) {


let's move this directly under waitForNoRelocatingShards

My bad. I fixed it.

dnhatn · 2017-11-22T19:25:29Z

@ywelsch I have addressed your comments. Could you please take another look? Thank you.

ywelsch · 2017-11-23T09:35:07Z

Left one more comment. Can you also update the documentation here:
https://www.elastic.co/guide/en/elasticsearch/reference/current/cluster-health.html

dnhatn · 2017-11-23T15:06:20Z

docs/reference/cluster/health.asciidoc

+    A boolean value which controls whether to wait (until the timeout provided)
+    for the cluster to have no shard initializations. Defaults to false, which means
+    it will not wait for initializing shards.
+


oh missed that, sorry.

ywelsch

LGTM

ywelsch · 2017-11-23T15:39:19Z

rest-api-spec/src/main/resources/rest-api-spec/test/cluster.health/10_basic.yml

+        body:
+          settings:
+            index:
+              number_of_shards: 50


The default of 5 is good enough (slow CI machines ftw)

Yep, I had to increase it to see the difference on my machine.

ywelsch · 2017-11-23T15:39:25Z

rest-api-spec/src/main/resources/rest-api-spec/test/cluster.health/10_basic.yml

+"cluster health basic test, one index with wait for no initializing shards":
+  - skip:
+      version: " - 6.99.99"
+      reason: "wait_for_no_initializing_shard is introduced in 7.0.0"


wait_for_no_initializing_shards

ywelsch · 2017-11-23T15:40:43Z

rest-api-spec/src/main/resources/rest-api-spec/test/cluster.health/10_basic.yml

+  - match:     { active_primary_shards:   50}
+  - gt:        { active_shards:           0 }
+  - gte:       { relocating_shards:       0 }
+  - match:     { initializing_shards:     0 }


I think it's good enough to just test this match. The other ones are covered by other tests in this same file.

dnhatn · 2017-11-23T20:09:43Z

Thanks @ywelsch.

This adds a new option to the cluster health request allowing to wait until there is no initializing shards. Closes #25623

Relates #27489

* es/master: (38 commits) Backport wait_for_initialiazing_shards to cluster health API Carry over version map size to prevent excessive resizing (#27516) Fix scroll query with a sort that is a prefix of the index sort (#27498) Delete shard store files before restoring a snapshot (#27476) Replace `delimited_payload_filter` by `delimited_payload` (#26625) CURRENT should not be a -SNAPSHOT version if build.snapshot is false (#27512) Fix merging of _meta field (#27352) Remove unused method (#27508) unmuted test, this has been fixed by #27397 Consolidate version numbering semantics (#27397) Add wait_for_no_initializing_shards to cluster health API (#27489) [TEST] use routing partition size based on the max routing shards of the second split Adjust CombinedDeletionPolicy for multiple commits (#27456) Update composite-aggregation.asciidoc Deprecate `levenstein` in favor of `levenshtein` (#27409) Automatically prepare indices for splitting (#27451) Validate `op_type` for `_create` (#27483) Minor ShapeBuilder cleanup muted test Decouple nio constructs from the tcp transport (#27484) ...

* es/6.x: (30 commits) Add wait_for_no_initializing_shards to cluster health API (#27489) Carry over version map size to prevent excessive resizing (#27516) Fix scroll query with a sort that is a prefix of the index sort (#27498) Delete shard store files before restoring a snapshot (#27476) CURRENT should not be a -SNAPSHOT version if build.snapshot is false (#27512) Fix merging of _meta field (#27352) test: do not run percolator query builder bwc test against 5.x versions Remove unused method (#27508) Consolidate version numbering semantics (#27397) Adjust CombinedDeletionPolicy for multiple commits (#27456) Minor ShapeBuilder cleanup [GEO] Deprecate ShapeBuilders and decouple geojson parse logic Improve docs for split API in 6.1/6.x (#27504) test: use correct pre 6.0.0-alpha1 format Update composite-aggregation.asciidoc Deprecate `levenstein` in favor of `levenshtein` (#27409) Decouple nio constructs from the tcp transport (#27484) Bump version from 6.1 to 6.2 Fix whitespace in Security.java Tighten which classes can exit ...

Add wait_for_no_initializing_shards to cluster health API

f5b32d4

This adds a new option to the cluster health request allowing to wait until there is no initializing shards. Closes elastic#25623

dnhatn added :Cluster >enhancement v6.1.0 v7.0.0 labels Nov 22, 2017

dnhatn requested review from ywelsch and jasontedor November 22, 2017 03:47

dnhatn added v6.2.0 and removed v6.1.0 labels Nov 22, 2017

Merge branch 'master' into cluster-health-api

79f6d3b

ywelsch suggested changes Nov 22, 2017

View reviewed changes

Address reviews

9f7e59c

Merge branch 'master' into cluster-health-api

4405767

dnhatn commented Nov 23, 2017

View reviewed changes

add a rest test

04e61fb

ywelsch approved these changes Nov 23, 2017

View reviewed changes

simplify the rest test

dd2294f

dnhatn merged commit 46b508d into elastic:master Nov 23, 2017

dnhatn added the backport pending label Nov 23, 2017

dnhatn added a commit that referenced this pull request Nov 24, 2017

Add wait_for_no_initializing_shards to cluster health API (#27489)

b5c3e08

This adds a new option to the cluster health request allowing to wait until there is no initializing shards. Closes #25623

dnhatn added a commit that referenced this pull request Nov 24, 2017

Backport wait_for_initialiazing_shards to cluster health API

06d35f4

Relates #27489

dnhatn deleted the cluster-health-api branch November 24, 2017 14:58

dnhatn removed the backport pending label Nov 24, 2017

jasontedor mentioned this pull request Jan 7, 2018

Add wait no initializing shards #26578

Closed

clintongormley added :Distributed/Distributed A catch all label for anything in the Distributed Area. If you aren't sure, use this one. and removed :Cluster labels Feb 13, 2018

codebrain mentioned this pull request May 23, 2018

[6.2] Adds wait_for_no_initializing_shards to cluster health API elastic/elasticsearch-net#3255

Closed

jimczi added v7.0.0-beta1 and removed v7.0.0 labels Feb 7, 2019

dliappis mentioned this pull request Jun 23, 2020

fix cluster health rest api wait_for_no_initializing_shards bug #58379

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds wait_for_no_initializing_shards to cluster health API #27489

Adds wait_for_no_initializing_shards to cluster health API #27489

dnhatn commented Nov 22, 2017

ywelsch left a comment

ywelsch Nov 22, 2017

dnhatn Nov 22, 2017

ywelsch Nov 22, 2017

dnhatn Nov 22, 2017

ywelsch Nov 22, 2017

dnhatn Nov 22, 2017

ywelsch Nov 23, 2017

dnhatn Nov 23, 2017

ywelsch Nov 22, 2017

dnhatn Nov 22, 2017

dnhatn commented Nov 22, 2017

ywelsch commented Nov 23, 2017

dnhatn Nov 23, 2017

ywelsch Nov 23, 2017

ywelsch left a comment

ywelsch Nov 23, 2017

dnhatn Nov 23, 2017

ywelsch Nov 23, 2017

ywelsch Nov 23, 2017

dnhatn commented Nov 23, 2017

Adds wait_for_no_initializing_shards to cluster health API #27489

Adds wait_for_no_initializing_shards to cluster health API #27489

Conversation

dnhatn commented Nov 22, 2017

ywelsch left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dnhatn commented Nov 22, 2017

ywelsch commented Nov 23, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ywelsch left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dnhatn commented Nov 23, 2017