Index creation waits for write consistency shards #18985

abeyad · 2016-06-20T20:41:11Z

Before returning, index creation now waits for the write consistency
number of shards to be available. An index can not take any indexing or
other replication operations without the write consistency level of
shards being available anyway, so waiting on the index creation response
in order for this condition to be met makes sense, and allows API users
to not depend on cluster health checks before attempting indexing
operations on the newly created index.

nik9000 · 2016-06-20T21:12:58Z

core/src/main/java/org/elasticsearch/action/admin/indices/create/CreateIndexRequest.java

@@ -77,6 +79,8 @@

    private boolean updateAllTypes = false;

+    private WriteConsistencyLevel waitOnShards = WriteConsistencyLevel.DEFAULT;


This seems like an odd name. Maybe call it waitForConsistency?

nik9000 · 2016-06-20T21:37:53Z

Wonderful news! I don't know a ton about the cluster state stuff but it seems sane and after applying it this gist passes:
https://gist.github.com/nik9000/3d65d5d883c7fd918a0835947c6549eb

ywelsch · 2016-06-22T09:35:39Z

.../src/main/java/org/elasticsearch/action/admin/indices/create/TransportCreateIndexAction.java

@@ -83,15 +94,34 @@ protected void masterOperation(final CreateIndexRequest request, final ClusterSt

            @Override
            public void onResponse(ClusterStateUpdateResponse response) {
-                listener.onResponse(new CreateIndexResponse(response.isAcknowledged()));
+                // the cluster state that includes the newly created index has been acknowledged,


I wonder if we should put this into MetaDataCreateIndexService instead (so we don't need to duplicate it into the two transport actions).

@ywelsch I actually did this in a new commit which I will push up soon.

ywelsch · 2016-06-24T08:00:32Z

core/src/main/java/org/elasticsearch/action/admin/indices/create/CreateIndexRequest.java

@@ -440,6 +443,28 @@ public CreateIndexRequest updateAllTypes(boolean updateAllTypes) {
        return this;
    }

+    public int waitOnActiveShards() {


There is a superclass ReplicationRequest where we should put these methods (as they apply to all the replication requests).

CreateIndexRequest does not extend ReplicationRequest, nor does UpdateRequest, etc.

bleskes · 2016-07-13T13:17:51Z

.../test/java/org/elasticsearch/action/admin/cluster/allocation/ClusterAllocationExplainIT.java

-                .put(IndexMetaData.INDEX_NUMBER_OF_REPLICAS_SETTING.getKey(), 1)).get();
-        ensureGreen("test");
+                .put(IndexMetaData.INDEX_NUMBER_OF_REPLICAS_SETTING.getKey(), 1))
+                .setWaitForActiveShards(ActiveShardCount.ALL).get();


ensure green has the nice reporting if we fail to get to green. Can we have an equivalent utility method that takes a create index response and check for shard acked, and log things? maybe a variant of assertAcked

bleskes · 2016-07-13T13:20:53Z

This looks great. I left some very minor comments. I do think we miss the change to TransportIndexAction and TransportBulkAction where we set the waitForActive shards to 0.

Of course, @ywelsch should also LGTM this as he is the main reviewer

abeyad · 2016-07-13T20:12:45Z

@bleskes Thank you for the review. 6c2e48a addresses your code review comments.

bleskes · 2016-07-14T09:32:27Z

core/src/main/java/org/elasticsearch/action/admin/indices/rollover/RolloverResponse.java

@@ -39,25 +39,31 @@
    private static final String DRY_RUN = "dry_run";
    private static final String ROLLED_OVER = "rolled_over";
    private static final String CONDITIONS = "conditions";
+    private static final String CREATE_INDEX_ACKNOWLEDGED = "create_index_acknowledged";


hmm.. i'm not sure we want to expose the complexity of the internal two steps. For example if the alias is acked and the index isn't, it doesn't really mean that the index isn't known by the nodes - having processed the alias change guarantees it. I think we can keep a simple acknowledge like in other responses, which means all nodes know about the new index and the new alias? it maybe then that with rollover the shards are acked but not the request. If we want to avoid that , I will only wait for shards active after the alias change is done.

it maybe then that with rollover the shards are acked but not the request. If we want to avoid that , I will only wait for shards active after the alias change is done.

That's kind of why its a bit nebulous. The semantics of acknowledged on ClusterStateUpdateResponse should be the same, but the trick here is that we have two different cluster state updates - one for index creation and one for the alias. We can move the shard waiting to after the alias swap, but then we also need to change the MetaDataCreateIndexService to make onlyCreateIndex public, and probably change the names.... createIndex only creates the index, createIndexAndWait includes the waiting... if they are to be public methods.. thoughts?

we can explicitly set the the waitForShards of the index creation to NONE and use the setting of the request to wait after the alias was acked. All variants of this are not ideal, so I would go for what you find the simplest - I tend to agree that consistency is the best here and thus leaning towards what I just described.

abeyad · 2016-07-14T15:44:13Z

@bleskes fcf60ee addresses the latest comments

ywelsch · 2016-07-15T14:07:44Z

core/src/main/java/org/elasticsearch/action/support/ActiveShardCount.java

+            case ALL_ACTIVE_SHARDS:
+                return ALL;
+            case 0:
+                return NONE;


we "special case" NONE here but not ONE, maybe it's simpler just to remove this method as well as the validateValue one and use new ActiveShardCount(...) in the two places it's currently used (and also ad

if (value < -2) { throw new IllegalArgumentException(...) }

to the constructor.

I you decide to go this route you should also remember to replace the reference equality checks (this == ActiveShardCount.NONE) by equals checks or by looking at value (this.value == 0).

The reason I preferred the current way is that we don't have to recreate new objects for what will most likely be the same values re-used over and over again.

ok sounds good. maybe add the case 1: then.

ywelsch · 2016-07-15T14:58:22Z

I left minor comments. Once addressed, feel free to push. LGTM! Thanks @abeyad.

Before returning, index creation now waits for the configured number of shard copies to be started. In the past, a client would create an index and then potentially have to check the cluster health to wait to execute write operations. With the cluster health semantics changing so that index creation does not cause the cluster health to go RED, this change enables waiting for the desired number of active shards to be active before returning from index creation. Relates elastic#9126

abeyad · 2016-07-15T15:17:50Z

@ywelsch @bleskes Thank you very much for all the valuable feedback!

abeyad added >enhancement resiliency :Cluster v5.0.0-alpha4 labels Jun 20, 2016

nik9000 reviewed Jun 20, 2016
View reviewed changes

ywelsch reviewed Jun 22, 2016
View reviewed changes

clintongormley removed the v5.0.0-alpha4 label Jun 22, 2016

abeyad force-pushed the index-creation-wait-for-shards branch 2 times, most recently from a99a910 to b511a5c Compare June 24, 2016 06:10

ywelsch reviewed Jun 24, 2016
View reviewed changes

bleskes reviewed Jul 13, 2016
View reviewed changes

bleskes reviewed Jul 14, 2016
View reviewed changes

ywelsch reviewed Jul 15, 2016
View reviewed changes

abeyad force-pushed the index-creation-wait-for-shards branch from fcf60ee to 6184ba3 Compare July 15, 2016 15:17

abeyad merged commit d78f40f into elastic:feature/friendly-index-creation Jul 15, 2016

This was referenced Jul 15, 2016

Makes index creation more friendly #19450

Merged

Removes write consistency level across replication action APIs in favor of wait_for_active_shards #19454

Merged

abeyad mentioned this pull request Jul 25, 2016

Documentation changes for wait_for_active_shards #19581

Merged

abeyad mentioned this pull request Sep 30, 2016

Parameter improvements to Cluster Health API wait for shards #20223

Merged

atombender mentioned this pull request Oct 31, 2016

Go through 5.0 breaking changes olivere/elastic#307

Closed

jpountz mentioned this pull request Nov 21, 2016

Bad performance of index creation on dockerized ES 5.0.1 #21692

Closed

tlrx mentioned this pull request Jan 10, 2017

cannot find ActiveShardCount #22505

Closed

pmusa mentioned this pull request Apr 25, 2017

WIP: Index creation waits for write consistency shards #18759

Closed

clintongormley added :Distributed Indexing/Distributed A catch all label for anything in the Distributed Area. Please avoid if you can. and removed :Cluster labels Feb 13, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Index creation waits for write consistency shards #18985

Index creation waits for write consistency shards #18985

abeyad commented Jun 20, 2016

nik9000 Jun 20, 2016

nik9000 commented Jun 20, 2016

ywelsch Jun 22, 2016

abeyad Jun 22, 2016

ywelsch Jun 24, 2016

abeyad Jun 24, 2016

bleskes Jul 13, 2016

bleskes commented Jul 13, 2016

abeyad commented Jul 13, 2016

bleskes Jul 14, 2016

abeyad Jul 14, 2016

bleskes Jul 14, 2016

abeyad commented Jul 14, 2016

ywelsch Jul 15, 2016

ywelsch Jul 15, 2016

abeyad Jul 15, 2016

ywelsch Jul 15, 2016

ywelsch commented Jul 15, 2016

abeyad commented Jul 15, 2016

		@@ -77,6 +79,8 @@

		private boolean updateAllTypes = false;

		private WriteConsistencyLevel waitOnShards = WriteConsistencyLevel.DEFAULT;

Index creation waits for write consistency shards #18985

Index creation waits for write consistency shards #18985

Conversation

abeyad commented Jun 20, 2016

Choose a reason for hiding this comment

nik9000 commented Jun 20, 2016

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

bleskes commented Jul 13, 2016

abeyad commented Jul 13, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abeyad commented Jul 14, 2016

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

ywelsch commented Jul 15, 2016

abeyad commented Jul 15, 2016