Skip to content

Commit

Permalink
Add ?wait_for_active_shards=default
Browse files Browse the repository at this point in the history
Today you cannot explicitly indicate that an operation should use the
usual behaviour of waiting for active shards according to the underlying
index setting. This is a problem for the close index API which has a
default of `none` in 7.x for BWC reasons (see elastic#33888), but the usual
behaviour in 8.0: you cannot today opt-in to the 8.0 behaviour with this
parameter.

This commit adds support for the literal value `default` for the
`wait_for_active_shards` query parameter.

Relates elastic#66419
  • Loading branch information
DaveCTurner committed Dec 18, 2020
1 parent cc15a28 commit 169f49f
Show file tree
Hide file tree
Showing 17 changed files with 27 additions and 32 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1008,7 +1008,7 @@ Params withWaitForActiveShards(ActiveShardCount activeShardCount) {
}

Params withWaitForActiveShards(ActiveShardCount activeShardCount, ActiveShardCount defaultActiveShardCount) {
if (activeShardCount != null && activeShardCount != defaultActiveShardCount) {
if (activeShardCount != null) {
return putParam("wait_for_active_shards", activeShardCount.toString().toLowerCase(Locale.ROOT));
}
return this;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ public void testPutFollow() throws Exception {
Request result = CcrRequestConverters.putFollow(putFollowRequest);
assertThat(result.getMethod(), equalTo(HttpPut.METHOD_NAME));
assertThat(result.getEndpoint(), equalTo("/" + putFollowRequest.getFollowerIndex() + "/_ccr/follow"));
if (putFollowRequest.waitForActiveShards() != null && putFollowRequest.waitForActiveShards() != ActiveShardCount.DEFAULT) {
if (putFollowRequest.waitForActiveShards() != null) {
String expectedValue = putFollowRequest.waitForActiveShards().toString().toLowerCase(Locale.ROOT);
assertThat(result.getParameters().get("wait_for_active_shards"), equalTo(expectedValue));
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
import org.elasticsearch.action.admin.cluster.health.ClusterHealthRequest;
import org.elasticsearch.action.admin.cluster.settings.ClusterGetSettingsRequest;
import org.elasticsearch.action.admin.cluster.settings.ClusterUpdateSettingsRequest;
import org.elasticsearch.action.support.ActiveShardCount;
import org.elasticsearch.action.support.master.AcknowledgedRequest;
import org.elasticsearch.client.cluster.RemoteInfoRequest;
import org.elasticsearch.cluster.health.ClusterHealthStatus;
Expand Down Expand Up @@ -104,7 +103,7 @@ public void testClusterHealth() {
default:
throw new UnsupportedOperationException();
}
RequestConvertersTests.setRandomWaitForActiveShards(healthRequest::waitForActiveShards, ActiveShardCount.NONE, expectedParams);
RequestConvertersTests.setRandomWaitForActiveShards(healthRequest::waitForActiveShards, expectedParams);
if (ESTestCase.randomBoolean()) {
ClusterHealthRequest.Level level = ESTestCase.randomFrom(ClusterHealthRequest.Level.values());
healthRequest.level(level);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -449,7 +449,7 @@ public void testReindex() throws IOException {
expectedParams.put("slices", "1");
}
setRandomTimeout(reindexRequest::setTimeout, ReplicationRequest.DEFAULT_TIMEOUT, expectedParams);
setRandomWaitForActiveShards(reindexRequest::setWaitForActiveShards, ActiveShardCount.DEFAULT, expectedParams);
setRandomWaitForActiveShards(reindexRequest::setWaitForActiveShards, expectedParams);
expectedParams.put("scroll", reindexRequest.getScrollTime().getStringRep());
expectedParams.put("wait_for_completion", Boolean.TRUE.toString());
Request request = RequestConverters.reindex(reindexRequest);
Expand Down Expand Up @@ -2053,24 +2053,19 @@ static void setRandomMasterTimeout(Consumer<TimeValue> setter, TimeValue default
}

static void setRandomWaitForActiveShards(Consumer<ActiveShardCount> setter, Map<String, String> expectedParams) {
setRandomWaitForActiveShards(setter, ActiveShardCount.DEFAULT, expectedParams);
}

static void setRandomWaitForActiveShards(Consumer<ActiveShardCount> setter, ActiveShardCount defaultActiveShardCount,
Map<String, String> expectedParams) {
if (randomBoolean()) {
int waitForActiveShardsInt = randomIntBetween(-1, 5);
int waitForActiveShardsInt = randomIntBetween(-2, 5);
String waitForActiveShardsString;
if (waitForActiveShardsInt == -1) {
waitForActiveShardsString = "all";
} else if (waitForActiveShardsInt == -2) {
waitForActiveShardsString = "default";
} else {
waitForActiveShardsString = String.valueOf(waitForActiveShardsInt);
}
ActiveShardCount activeShardCount = ActiveShardCount.parseString(waitForActiveShardsString);
setter.accept(activeShardCount);
if (defaultActiveShardCount.equals(activeShardCount) == false) {
expectedParams.put("wait_for_active_shards", waitForActiveShardsString);
}
expectedParams.put("wait_for_active_shards", waitForActiveShardsString);
}
}

Expand Down
8 changes: 4 additions & 4 deletions docs/reference/docs/index_.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -353,10 +353,10 @@ This default can be overridden in the index settings dynamically
by setting `index.write.wait_for_active_shards`. To alter this behavior
per operation, the `wait_for_active_shards` request parameter can be used.

Valid values are `all` or any positive integer up to the total number
of configured copies per shard in the index (which is `number_of_replicas+1`).
Specifying a negative value or a number greater than the number of
shard copies will throw an error.
Valid values are `all`, `default`, and any positive integer up to the total
number of configured copies per shard in the index (which is
`number_of_replicas+1`). Specifying a negative value or a number greater than
the number of shard copies will throw an error.

For example, suppose we have a cluster of three nodes, `A`, `B`, and `C` and
we create an index `index` with the number of replicas set to 3 (resulting in
Expand Down
7 changes: 4 additions & 3 deletions docs/reference/rest-api/common-parms.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -1073,9 +1073,10 @@ tag::wait_for_active_shards[]
+
--
(Optional, string) The number of shard copies that must be active before
proceeding with the operation. Set to `all` or any positive integer up
to the total number of shards in the index (`number_of_replicas+1`).
Default: 1, the primary shard.
proceeding with the operation. Set to `all` to wait for all shard copies to be
active, or `default` to use the value from the
`index.write.wait_for_active_shards` setting from the underlying index, or an
integer between `1` and `number_of_replicas+1`. Defaults to `default`.

See <<index-wait-for-active-shards>>.
--
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@
"params":{
"wait_for_active_shards":{
"type":"string",
"description":"Sets the number of shard copies that must be active before proceeding with the bulk operation. Defaults to 1, meaning the primary shard only. Set to `all` for all shard copies, otherwise set to any non-negative value less than or equal to the total number of copies for the shard (number of replicas + 1)"
"description":"Sets the number of shard copies that must be active before proceeding with the bulk operation. Defaults to `default` meaning to use the value of the `index.write.wait_for_active_shards` setting. Set to `all` for all shard copies, otherwise set to any non-negative value less than or equal to the total number of copies for the shard (number of replicas + 1)."
},
"refresh":{
"type":"enum",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@
"params":{
"wait_for_active_shards":{
"type":"string",
"description":"Sets the number of shard copies that must be active before proceeding with the index operation. Defaults to 1, meaning the primary shard only. Set to `all` for all shard copies, otherwise set to any non-negative value less than or equal to the total number of copies for the shard (number of replicas + 1)"
"description":"Sets the number of shard copies that must be active before proceeding with the index operation. Defaults to `default` meaning to use the value of the `index.write.wait_for_active_shards` setting. Set to `all` for all shard copies, otherwise set to any non-negative value less than or equal to the total number of copies for the shard (number of replicas + 1)."
},
"refresh":{
"type":"enum",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@
"params":{
"wait_for_active_shards":{
"type":"string",
"description":"Sets the number of shard copies that must be active before proceeding with the delete operation. Defaults to 1, meaning the primary shard only. Set to `all` for all shard copies, otherwise set to any non-negative value less than or equal to the total number of copies for the shard (number of replicas + 1)"
"description":"Sets the number of shard copies that must be active before proceeding with the delete operation. Defaults to `default` meaning to use the value of the `index.write.wait_for_active_shards` setting. Set to `all` for all shard copies, otherwise set to any non-negative value less than or equal to the total number of copies for the shard (number of replicas + 1)."
},
"refresh":{
"type":"enum",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@
},
"wait_for_active_shards":{
"type":"string",
"description":"Sets the number of shard copies that must be active before proceeding with the delete by query operation. Defaults to 1, meaning the primary shard only. Set to `all` for all shard copies, otherwise set to any non-negative value less than or equal to the total number of copies for the shard (number of replicas + 1)"
"description":"Sets the number of shard copies that must be active before proceeding with the delete by query operation. Defaults to `default` meaning to use the value of the `index.write.wait_for_active_shards` setting. Set to `all` for all shard copies, otherwise set to any non-negative value less than or equal to the total number of copies for the shard (number of replicas + 1)."
},
"scroll_size":{
"type":"number",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@
"params":{
"wait_for_active_shards":{
"type":"string",
"description":"Sets the number of shard copies that must be active before proceeding with the index operation. Defaults to 1, meaning the primary shard only. Set to `all` for all shard copies, otherwise set to any non-negative value less than or equal to the total number of copies for the shard (number of replicas + 1)"
"description":"Sets the number of shard copies that must be active before returning. Defaults to `default` meaning to use the value of the `index.write.wait_for_active_shards` setting. Set to `all` for all shard copies, otherwise set to any non-negative value less than or equal to the total number of copies for the shard (number of replicas + 1)."
},
"op_type":{
"type":"enum",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
},
"wait_for_active_shards":{
"type":"string",
"description":"Sets the number of shard copies that must be active before proceeding with the reindex operation. Defaults to 1, meaning the primary shard only. Set to `all` for all shard copies, otherwise set to any non-negative value less than or equal to the total number of copies for the shard (number of replicas + 1)"
"description":"Sets the number of shard copies that must be active before proceeding with the reindex operation. Defaults to `default` meaning to use the value of the `index.write.wait_for_active_shards` setting. Set to `all` for all shard copies, otherwise set to any non-negative value less than or equal to the total number of copies for the shard (number of replicas + 1)."
},
"wait_for_completion":{
"type":"boolean",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@
"params":{
"wait_for_active_shards":{
"type":"string",
"description":"Sets the number of shard copies that must be active before proceeding with the update operation. Defaults to 1, meaning the primary shard only. Set to `all` for all shard copies, otherwise set to any non-negative value less than or equal to the total number of copies for the shard (number of replicas + 1)"
"description":"Sets the number of shard copies that must be active before proceeding with the update operation. Defaults to `default` meaning to use the value of the `index.write.wait_for_active_shards` setting. Set to `all` for all shard copies, otherwise set to any non-negative value less than or equal to the total number of copies for the shard (number of replicas + 1)."
},
"_source":{
"type":"list",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@
},
"wait_for_active_shards":{
"type":"string",
"description":"Sets the number of shard copies that must be active before proceeding with the update by query operation. Defaults to 1, meaning the primary shard only. Set to `all` for all shard copies, otherwise set to any non-negative value less than or equal to the total number of copies for the shard (number of replicas + 1)"
"description":"Sets the number of shard copies that must be active before proceeding with the update by query operation. Defaults to `default` meaning to use the value of the `index.write.wait_for_active_shards` setting. Set to `all` for all shard copies, otherwise set to any non-negative value less than or equal to the total number of copies for the shard (number of replicas + 1)."
},
"scroll_size":{
"type":"number",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ public static ActiveShardCount readFrom(final StreamInput in) throws IOException
* IllegalArgumentException.
*/
public static ActiveShardCount parseString(final String str) {
if (str == null) {
if (str == null || str.equals("default")) {
return ActiveShardCount.DEFAULT;
} else if (str.equals("all")) {
return ActiveShardCount.ALL;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ public void testSerialization() throws IOException {
public void testParseString() {
assertSame(ActiveShardCount.parseString("all"), ActiveShardCount.ALL);
assertSame(ActiveShardCount.parseString(null), ActiveShardCount.DEFAULT);
assertSame(ActiveShardCount.parseString("default"), ActiveShardCount.DEFAULT);
assertSame(ActiveShardCount.parseString("0"), ActiveShardCount.NONE);
int value = randomIntBetween(1, 50);
assertEquals(ActiveShardCount.parseString(value + ""), ActiveShardCount.from(value));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,8 +130,7 @@ public ActiveShardCount waitForActiveShards() {
/**
* Sets the number of shard copies that should be active for follower index creation to
* return. Defaults to {@link ActiveShardCount#NONE}, which will not wait for any shards
* to be active. Set this value to {@link ActiveShardCount#DEFAULT} to wait for the primary
* shard to be active. Set this value to {@link ActiveShardCount#ALL} to wait for all shards
* to be active. Set this value to {@link ActiveShardCount#ALL} to wait for all shards
* (primary and all replicas) to be active before returning.
*
* @param waitForActiveShards number of active shard copies to wait on
Expand Down

0 comments on commit 169f49f

Please sign in to comment.