Skip to content

Commit

Permalink
Warn of change of default of wait_for_active_shards (#67246)
Browse files Browse the repository at this point in the history
In 7.x the close indices API defaults to `?wait_for_active_shards=0` but
from 8.0 it will default to respecting the index settings instead. This
commit introduces the `index-setting` value for this parameter on this
API allowing users to opt-in to the future behaviour today, and starts
to emit a deprecation warning for users that use the default.

Relates #67158
Closes #66419
  • Loading branch information
DaveCTurner committed Jan 13, 2021
1 parent 345dd82 commit 8660e8d
Show file tree
Hide file tree
Showing 58 changed files with 308 additions and 42 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
import org.elasticsearch.action.admin.indices.shrink.ResizeType;
import org.elasticsearch.action.admin.indices.template.delete.DeleteIndexTemplateRequest;
import org.elasticsearch.action.admin.indices.validate.query.ValidateQueryRequest;
import org.elasticsearch.action.support.ActiveShardCount;
import org.elasticsearch.client.indices.AnalyzeRequest;
import org.elasticsearch.client.indices.CloseIndexRequest;
import org.elasticsearch.client.indices.CreateDataStreamRequest;
Expand Down Expand Up @@ -140,6 +141,13 @@ static Request closeIndex(CloseIndexRequest closeIndexRequest) {
parameters.withTimeout(closeIndexRequest.timeout());
parameters.withMasterTimeout(closeIndexRequest.masterNodeTimeout());
parameters.withIndicesOptions(closeIndexRequest.indicesOptions());

final ActiveShardCount activeShardCount = closeIndexRequest.waitForActiveShards();
if (activeShardCount == ActiveShardCount.DEFAULT) {
request.addParameter("wait_for_active_shards", "index-setting");
} else {
parameters.withWaitForActiveShards(activeShardCount);
}
request.addParameters(parameters.asMap());
return request;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@

package org.elasticsearch.client.indices;

import org.elasticsearch.action.admin.indices.open.OpenIndexResponse;
import org.elasticsearch.action.support.ActiveShardCount;
import org.elasticsearch.action.support.IndicesOptions;
import org.elasticsearch.client.TimedRequest;
Expand All @@ -35,7 +34,7 @@ public class CloseIndexRequest extends TimedRequest implements Validatable {

private String[] indices;
private IndicesOptions indicesOptions = IndicesOptions.strictExpandOpen();
private ActiveShardCount waitForActiveShards = ActiveShardCount.DEFAULT;
private ActiveShardCount waitForActiveShards = null;

/**
* Creates a new close index request
Expand Down Expand Up @@ -82,16 +81,14 @@ public ActiveShardCount waitForActiveShards() {
}

/**
* Sets the number of shard copies that should be active for indices opening to return.
* Defaults to {@link ActiveShardCount#DEFAULT}, which will wait for one shard copy
* (the primary) to become active. Set this value to {@link ActiveShardCount#ALL} to
* wait for all shards (primary and all replicas) to be active before returning.
* Otherwise, use {@link ActiveShardCount#from(int)} to set this value to any
* non-negative integer, up to the number of copies per shard (number of replicas + 1),
* to wait for the desired amount of shard copies to become active before returning.
* Indices opening will only wait up until the timeout value for the number of shard copies
* to be active before returning. Check {@link OpenIndexResponse#isShardsAcknowledged()} to
* determine if the requisite shard copies were all started before returning or timing out.
* Sets the number of shard copies that should be active before a close-index request returns. Defaults to {@code null}, which means not
* to wait. However the default behaviour is deprecated and will change in version 8. You can opt-in to the new default behaviour now by
* setting this to {@link ActiveShardCount#DEFAULT}, which will wait according to the setting {@code index.write.wait_for_active_shards}
* which by default will wait for one shard, the primary. Set this value to {@link ActiveShardCount#ALL} to wait for all shards (primary
* and all replicas) to be active before returning. Otherwise, use {@link ActiveShardCount#from(int)} to set this value to any
* non-negative integer up to the number of copies per shard (number of replicas + 1), to wait for the desired amount of shard copies
* to become active before returning. To explicitly preserve today's default behaviour and suppress the deprecation warning, set this
* property to {@code ActiveShardCount.from(0)}.
*
* @param waitForActiveShards number of active shard copies to wait on
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,7 @@ public void testIndexFollowing() throws Exception {

// Need to close index prior to unfollowing it:
CloseIndexRequest closeIndexRequest = new CloseIndexRequest("follower");
closeIndexRequest.waitForActiveShards(ActiveShardCount.from(0));
org.elasticsearch.action.support.master.AcknowledgedResponse closeIndexReponse =
highLevelClient().indices().close(closeIndexRequest, RequestOptions.DEFAULT);
assertThat(closeIndexReponse.isAcknowledged(), is(true));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
import org.elasticsearch.action.admin.indices.validate.query.ValidateQueryRequest;
import org.elasticsearch.action.admin.indices.validate.query.ValidateQueryResponse;
import org.elasticsearch.action.index.IndexRequest;
import org.elasticsearch.action.support.ActiveShardCount;
import org.elasticsearch.action.support.IndicesOptions;
import org.elasticsearch.action.support.WriteRequest;
import org.elasticsearch.action.support.broadcast.BroadcastResponse;
Expand Down Expand Up @@ -904,6 +905,7 @@ public void testCloseExistingIndex() throws IOException {
}

CloseIndexRequest closeIndexRequest = new CloseIndexRequest(indices);
closeIndexRequest.waitForActiveShards(ActiveShardCount.from(0));
CloseIndexResponse closeIndexResponse = execute(closeIndexRequest,
highLevelClient().indices()::close, highLevelClient().indices()::closeAsync);
assertTrue(closeIndexResponse.isAcknowledged());
Expand All @@ -926,6 +928,7 @@ public void testCloseNonExistentIndex() throws IOException {
assertFalse(indexExists(nonExistentIndex));

CloseIndexRequest closeIndexRequest = new CloseIndexRequest(nonExistentIndex);
closeIndexRequest.waitForActiveShards(ActiveShardCount.from(0));
ElasticsearchException exception = expectThrows(ElasticsearchException.class,
() -> execute(closeIndexRequest, highLevelClient().indices()::close, highLevelClient().indices()::closeAsync));
assertEquals(RestStatus.NOT_FOUND, exception.status());
Expand All @@ -934,6 +937,7 @@ public void testCloseNonExistentIndex() throws IOException {
public void testCloseEmptyOrNullIndex() {
String[] indices = randomBoolean() ? Strings.EMPTY_ARRAY : null;
CloseIndexRequest closeIndexRequest = new CloseIndexRequest(indices);
closeIndexRequest.waitForActiveShards(ActiveShardCount.from(0));
org.elasticsearch.client.ValidationException exception = expectThrows(org.elasticsearch.client.ValidationException.class,
() -> execute(closeIndexRequest, highLevelClient().indices()::close, highLevelClient().indices()::closeAsync));
assertThat(exception.validationErrors().get(0), equalTo("index is missing"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
import org.elasticsearch.action.admin.indices.shrink.ResizeType;
import org.elasticsearch.action.admin.indices.template.delete.DeleteIndexTemplateRequest;
import org.elasticsearch.action.admin.indices.validate.query.ValidateQueryRequest;
import org.elasticsearch.action.support.ActiveShardCount;
import org.elasticsearch.action.support.master.AcknowledgedRequest;
import org.elasticsearch.client.indices.AnalyzeRequest;
import org.elasticsearch.client.indices.CloseIndexRequest;
Expand Down Expand Up @@ -641,6 +642,22 @@ public void testCloseIndex() {
RequestConvertersTests.setRandomMasterTimeout(closeIndexRequest, expectedParams);
RequestConvertersTests.setRandomIndicesOptions(closeIndexRequest::indicesOptions, closeIndexRequest::indicesOptions,
expectedParams);
switch (between(0, 3)) {
case 0:
break;
case 1:
closeIndexRequest.waitForActiveShards(ActiveShardCount.DEFAULT);
expectedParams.put("wait_for_active_shards", "index-setting");
break;
case 2:
closeIndexRequest.waitForActiveShards(ActiveShardCount.ALL);
expectedParams.put("wait_for_active_shards", "all");
break;
case 3:
closeIndexRequest.waitForActiveShards(ActiveShardCount.from(1));
expectedParams.put("wait_for_active_shards", "1");
break;
}

Request request = IndicesRequestConverters.closeIndex(closeIndexRequest);
StringJoiner endpoint = new StringJoiner("/", "/", "").add(String.join(",", indices)).add("_close");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ public void testRankEvalRequest() throws IOException {
}

// now try this when test2 is closed
client().performRequest(new Request("POST", "index2/_close"));
closeIndex("index2");
rankEvalRequest.indicesOptions(IndicesOptions.fromParameters(null, "true", null, "false", SearchRequest.DEFAULT_INDICES_OPTIONS));
response = execute(rankEvalRequest, highLevelClient()::rankEval, highLevelClient()::rankEvalAsync);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -321,6 +321,7 @@ public void testUnfollow() throws Exception {
assertThat(unfollowResponse.isAcknowledged(), is(true));

CloseIndexRequest closeIndexRequest = new CloseIndexRequest(followIndex);
closeIndexRequest.waitForActiveShards(ActiveShardCount.from(0));
assertThat(client.indices().close(closeIndexRequest, RequestOptions.DEFAULT).isAcknowledged(), is(true));
}

Expand Down Expand Up @@ -353,6 +354,7 @@ public void testUnfollow() throws Exception {
assertThat(unfollowResponse.isAcknowledged(), is(true));

CloseIndexRequest closeIndexRequest = new CloseIndexRequest(followIndex);
closeIndexRequest.waitForActiveShards(ActiveShardCount.from(0));
assertThat(client.indices().close(closeIndexRequest, RequestOptions.DEFAULT).isAcknowledged(), is(true));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1469,6 +1469,8 @@ public void testCloseIndex() throws Exception {
CloseIndexRequest request = new CloseIndexRequest("index"); // <1>
// end::close-index-request

request.waitForActiveShards(ActiveShardCount.from(0));

// tag::close-index-request-timeout
request.setTimeout(TimeValue.timeValueMinutes(2)); // <1>
// end::close-index-request-timeout
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,9 @@ private void closeWhileListenerEngaged(ActionFuture<String> future) throws Excep
});

// Close the index. That should flush the listener.
client().performRequest(new Request("POST", "/test/_close"));
final Request closeRequest = new Request("POST", "/test/_close");
closeRequest.addParameter("wait_for_active_shards", "0");
client().performRequest(closeRequest);

/*
* The request may fail, but we really, really, really want to make
Expand Down
2 changes: 1 addition & 1 deletion docs/reference/ccr/apis/follow/post-unfollow.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ PUT /follower_index/_ccr/follow?wait_for_active_shards=1
POST /follower_index/_ccr/pause_follow
POST /follower_index/_close
POST /follower_index/_close?wait_for_active_shards=0
--------------------------------------------------
// TESTSETUP
// TEST[setup:remote_cluster_and_leader_index]
Expand Down
2 changes: 1 addition & 1 deletion docs/reference/ccr/getting-started.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ replication
--------------------------------------------------
POST /server-metrics-follower/_ccr/pause_follow
POST /server-metrics-follower/_close
POST /server-metrics-follower/_close?wait_for_active_shards=0
POST /server-metrics-follower/_ccr/unfollow
--------------------------------------------------
Expand Down
2 changes: 1 addition & 1 deletion docs/reference/ccr/managing.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ process. Then, close the follower index and recreate it. For example:
----------------------------------------------------------------------
POST /follower_index/_ccr/pause_follow
POST /follower_index/_close
POST /follower_index/_close?wait_for_active_shards=0
PUT /follower_index/_ccr/follow?wait_for_active_shards=1
{
Expand Down
2 changes: 1 addition & 1 deletion docs/reference/index-modules/similarity.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -540,7 +540,7 @@ request and <<indices-open-close,open>> it again afterwards:

[source,console]
--------------------------------------------------
POST /index/_close
POST /index/_close?wait_for_active_shards=0
PUT /index/_settings
{
Expand Down
17 changes: 15 additions & 2 deletions docs/reference/indices/close.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ Closes an index.
POST /my-index-000001/_close
--------------------------------------------------
// TEST[setup:my_index]
// TEST[warning:the default value for the ?wait_for_active_shards parameter will change from '0' to 'index-setting' in version 8; specify '?wait_for_active_shards=index-setting' to adopt the future default behaviour, or '?wait_for_active_shards=0' to preserve today's behaviour]


[[close-index-api-request]]
Expand Down Expand Up @@ -52,7 +53,18 @@ Defaults to `open`.

include::{es-repo-dir}/rest-api/common-parms.asciidoc[tag=index-ignore-unavailable]

include::{es-repo-dir}/rest-api/common-parms.asciidoc[tag=wait_for_active_shards]
`wait_for_active_shards`::
+
--
(Optional, string) The number of shard copies that must be active before
proceeding with the operation. Set to `all`, `index-setting`, or any positive
integer up to the total number of shards in the index (`number_of_replicas+1`).
The value `index-setting` means to wait according to the index setting
`index.write.wait_for_active_shards`. Default: `0`, meaning do not wait for any
shards to be ready.

See <<index-wait-for-active-shards>>.
--

include::{es-repo-dir}/rest-api/common-parms.asciidoc[tag=timeoutparms]

Expand All @@ -66,7 +78,8 @@ The following example shows how to close an index:
--------------------------------------------------
POST /my-index-000001/_close
--------------------------------------------------
// TEST[s/^/PUT my-index-000001\n/]
// TEST[setup:my_index]
// TEST[warning:the default value for the ?wait_for_active_shards parameter will change from '0' to 'index-setting' in version 8; specify '?wait_for_active_shards=index-setting' to adopt the future default behaviour, or '?wait_for_active_shards=0' to preserve today's behaviour]

The API returns following response:

Expand Down
4 changes: 2 additions & 2 deletions docs/reference/indices/open-close.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ opens any closed backing indices.
POST /my-index-000001/_open
--------------------------------------------------
// TEST[setup:my_index]
// TEST[s/^/POST \/my-index-000001\/_close\n/]
// TEST[s/^/POST \/my-index-000001\/_close?wait_for_active_shards=0\n/]


[[open-index-api-request]]
Expand Down Expand Up @@ -122,7 +122,7 @@ The following request re-opens a closed index named `my-index-000001`.
--------------------------------------------------
POST /my-index-000001/_open
--------------------------------------------------
// TEST[s/^/PUT my-index-000001\nPOST my-index-000001\/_close\n/]
// TEST[s/^/PUT my-index-000001\nPOST my-index-000001\/_close?wait_for_active_shards=0\n/]

The API returns the following response:

Expand Down
2 changes: 1 addition & 1 deletion docs/reference/indices/resolve.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ supported.
----
PUT /foo_closed
POST /foo_closed/_close
POST /foo_closed/_close?wait_for_active_shards=0
PUT /remotecluster-bar-01
Expand Down
2 changes: 1 addition & 1 deletion docs/reference/indices/update-settings.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ the following commands add the `content` analyzer to the `my-index-000001` index

[source,console]
--------------------------------------------------
POST /my-index-000001/_close
POST /my-index-000001/_close?wait_for_active_shards=0
PUT /my-index-000001/_settings
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,13 @@ PUT _snapshot/my_repository/snapshot_2?wait_for_completion=true
}
}
POST /index_1/_close
POST /index_1/_close?wait_for_active_shards=0
POST /index_2/_close
POST /index_2/_close?wait_for_active_shards=0
POST /index_3/_close
POST /index_3/_close?wait_for_active_shards=0
POST /index_4/_close
POST /index_4/_close?wait_for_active_shards=0
----
// TESTSETUP
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
#
---
"Create a snapshot and then restore it":
- skip:
features: ["allowed_warnings"]

# Create repository
- do:
Expand Down Expand Up @@ -47,6 +49,8 @@
- do:
indices.close:
index : test_index
allowed_warnings:
- "the default value for the ?wait_for_active_shards parameter will change from '0' to 'index-setting' in version 8; specify '?wait_for_active_shards=index-setting' to adopt the future default behaviour, or '?wait_for_active_shards=0' to preserve today's behaviour"

# Restore index
- do:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
#
---
"Create a snapshot and then restore it":
- skip:
features: ["allowed_warnings"]

# Create repository
- do:
Expand Down Expand Up @@ -49,6 +51,8 @@
- do:
indices.close:
index : test_index
allowed_warnings:
- "the default value for the ?wait_for_active_shards parameter will change from '0' to 'index-setting' in version 8; specify '?wait_for_active_shards=index-setting' to adopt the future default behaviour, or '?wait_for_active_shards=0' to preserve today's behaviour"

# Restore index
- do:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@
},
"wait_for_active_shards":{
"type":"string",
"description":"Sets the number of active shards to wait for before the operation returns."
"description":"Sets the number of active shards to wait for before the operation returns. Set to `index-setting` to wait according to the index setting `index.write.wait_for_active_shards`, or `all` to wait for all shards, or an integer. Defaults to `0`."
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -388,6 +388,7 @@
- skip:
version: " - 7.3.99"
reason: "is_write_index is shown in cat.aliases starting version 7.4.0"
features: ["allowed_warnings"]

- do:
indices.create:
Expand All @@ -399,6 +400,8 @@
- do:
indices.close:
index: test_index
allowed_warnings:
- "the default value for the ?wait_for_active_shards parameter will change from '0' to 'index-setting' in version 8; specify '?wait_for_active_shards=index-setting' to adopt the future default behaviour, or '?wait_for_active_shards=0' to preserve today's behaviour"

- do:
cat.aliases:
Expand All @@ -419,7 +422,7 @@
"Alias against closed index (pre 7.4.0)":
- skip:
version: "7.4.0 - "
features: node_selector
features: ["node_selector", "allowed_warnings"]
reason: "is_write_index is shown in cat.aliases starting version 7.4.0"

- do:
Expand All @@ -432,6 +435,8 @@
- do:
indices.close:
index: test_index
allowed_warnings:
- "the default value for the ?wait_for_active_shards parameter will change from '0' to 'index-setting' in version 8; specify '?wait_for_active_shards=index-setting' to adopt the future default behaviour, or '?wait_for_active_shards=0' to preserve today's behaviour"

- do:
node_selector:
Expand Down
Loading

0 comments on commit 8660e8d

Please sign in to comment.