Skip to content

Commit

Permalink
Remove support for controversial ignore_unavailable and allow_no_indi…
Browse files Browse the repository at this point in the history
…ces from indices exists api

Exist requests are supposed to never throw an exception, but rather return true or false depending on whether some resource exists or not. Indices exists does that for indices and accepts wildcard expressions too. The way the api works internally is by resolving indices and catching IndexNotFoundException: if an exception is thrown the index does not exist hence it returns false, otherwise it returns true. That works ok only if ignore_unavailable and allow_no_indices indices options are both set to false, meaning that they are strict and any missing index or wildcard expressions that resolves to no indices will lead to an exception that can be thrown and cause false to be returned.

Unfortunately the indices options have  been configurable up until now for this request, meaning that one can set ignore_unavailable or allow_no_indices to true and have the indices exist request return true for indices that really don't exist, which makes very little sense in the context of this api.

This commit removes the indicesOptions setter from the IndicesExistsRequest and makes settable only expandWildcardsOpen and expandWildcardsClosed, hence a subset of the available indices options. This way we can guarantee more consistent behaviour of the indices exists api. We can then remove the ignore_unavailable and allow_no_indices option from indices exists api spec
  • Loading branch information
javanna committed Nov 2, 2016
1 parent b3370de commit 7120587
Show file tree
Hide file tree
Showing 6 changed files with 48 additions and 23 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,15 @@ public IndicesOptions indicesOptions() {
return indicesOptions;
}

public IndicesExistsRequest indicesOptions(IndicesOptions indicesOptions) {
this.indicesOptions = indicesOptions;
public IndicesExistsRequest expandWilcardsOpen(boolean expandWildcardsOpen) {
this.indicesOptions = IndicesOptions.fromOptions(indicesOptions.ignoreUnavailable(), indicesOptions.allowNoIndices(),
expandWildcardsOpen, indicesOptions.expandWildcardsClosed());
return this;
}

public IndicesExistsRequest expandWilcardsClosed(boolean expandWildcardsClosed) {
this.indicesOptions = IndicesOptions.fromOptions(indicesOptions.ignoreUnavailable(), indicesOptions.allowNoIndices(),
indicesOptions.expandWildcardsOpen(), expandWildcardsClosed);
return this;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@

package org.elasticsearch.action.admin.indices.exists.indices;

import org.elasticsearch.action.support.IndicesOptions;
import org.elasticsearch.action.support.master.MasterNodeReadOperationRequestBuilder;
import org.elasticsearch.client.ElasticsearchClient;

Expand All @@ -35,12 +34,18 @@ public IndicesExistsRequestBuilder setIndices(String... indices) {
}

/**
* Specifies what type of requested indices to ignore and wildcard indices expressions.
* <p>
* For example indices that don't exist.
* Controls whether wildcard expressions will be expanded to existing open indices
*/
public IndicesExistsRequestBuilder setIndicesOptions(IndicesOptions options) {
request.indicesOptions(options);
public IndicesExistsRequestBuilder setExpandWildcardsOpen(boolean expandWildcardsOpen) {
request.expandWilcardsOpen(expandWildcardsOpen);
return this;
}

/**
* Controls whether wildcard expressions will be expanded to existing closed indices
*/
public IndicesExistsRequestBuilder setExpandWildcardsClosed(boolean expandWildcardsClosed) {
request.expandWilcardsClosed(expandWildcardsClosed);
return this;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,9 @@ public RestIndicesExistsAction(Settings settings, RestController controller) {
@Override
public RestChannelConsumer prepareRequest(final RestRequest request, final NodeClient client) throws IOException {
IndicesExistsRequest indicesExistsRequest = new IndicesExistsRequest(Strings.splitStringByCommaToArray(request.param("index")));
indicesExistsRequest.indicesOptions(IndicesOptions.fromRequest(request, indicesExistsRequest.indicesOptions()));
IndicesOptions indicesOptions = IndicesOptions.fromRequest(request, indicesExistsRequest.indicesOptions());
indicesExistsRequest.expandWilcardsOpen(indicesOptions.expandWildcardsOpen());
indicesExistsRequest.expandWilcardsClosed(indicesOptions.expandWildcardsClosed());
indicesExistsRequest.local(request.paramAsBoolean("local", indicesExistsRequest.local()));
return channel -> client.admin().indices().exists(indicesExistsRequest, new RestResponseListener<IndicesExistsResponse>(channel) {
@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@

package org.elasticsearch.indices.exists.indices;

import org.elasticsearch.action.support.IndicesOptions;
import org.elasticsearch.action.admin.indices.exists.indices.IndicesExistsRequest;
import org.elasticsearch.action.admin.indices.exists.indices.IndicesExistsRequestBuilder;
import org.elasticsearch.cluster.block.ClusterBlockException;
import org.elasticsearch.cluster.metadata.IndexMetaData;
import org.elasticsearch.test.ESIntegTestCase;
Expand All @@ -29,21 +30,33 @@
import static org.elasticsearch.cluster.metadata.IndexMetaData.SETTING_BLOCKS_READ;
import static org.elasticsearch.cluster.metadata.IndexMetaData.SETTING_BLOCKS_WRITE;
import static org.elasticsearch.cluster.metadata.IndexMetaData.SETTING_READ_ONLY;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
import static org.hamcrest.Matchers.equalTo;

public class IndicesExistsIT extends ESIntegTestCase {
// Indices exists never throws IndexMissingException, the indices options control its behaviour (return true or false)
public void testIndicesExists() throws Exception {
assertThat(client().admin().indices().prepareExists("foo").get().isExists(), equalTo(false));
assertThat(client().admin().indices().prepareExists("foo").setIndicesOptions(IndicesOptions.lenientExpandOpen()).get().isExists(), equalTo(true));
assertThat(client().admin().indices().prepareExists("foo*").get().isExists(), equalTo(false));
assertThat(client().admin().indices().prepareExists("foo*").setIndicesOptions(IndicesOptions.fromOptions(false, true, true, false)).get().isExists(), equalTo(true));
assertThat(client().admin().indices().prepareExists("_all").get().isExists(), equalTo(false));
assertFalse(client().admin().indices().prepareExists("foo").get().isExists());
assertFalse(client().admin().indices().prepareExists("foo*").get().isExists());
assertFalse(client().admin().indices().prepareExists("_all").get().isExists());

createIndex("foo", "foobar", "bar", "barbaz");

IndicesExistsRequestBuilder indicesExistsRequestBuilder = client().admin().indices().prepareExists("foo*")
.setExpandWildcardsOpen(false);
IndicesExistsRequest request = indicesExistsRequestBuilder.request();
//check that ignore unavailable and allow no indices are set to false. That is their only valid value as it can't be overridden
assertFalse(request.indicesOptions().ignoreUnavailable());
assertFalse(request.indicesOptions().allowNoIndices());
assertThat(indicesExistsRequestBuilder.get().isExists(), equalTo(false));

assertAcked(client().admin().indices().prepareClose("foobar").get());

assertThat(client().admin().indices().prepareExists("foo*").get().isExists(), equalTo(true));
assertThat(client().admin().indices().prepareExists("foo*").setExpandWildcardsOpen(false)
.setExpandWildcardsClosed(false).get().isExists(), equalTo(false));
assertThat(client().admin().indices().prepareExists("foobar").get().isExists(), equalTo(true));
assertThat(client().admin().indices().prepareExists("foob*").setExpandWildcardsClosed(false).get().isExists(), equalTo(false));
assertThat(client().admin().indices().prepareExists("bar*").get().isExists(), equalTo(true));
assertThat(client().admin().indices().prepareExists("bar").get().isExists(), equalTo(true));
assertThat(client().admin().indices().prepareExists("_all").get().isExists(), equalTo(true));
Expand Down
6 changes: 6 additions & 0 deletions docs/reference/migration/migrate_6_0/rest.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,9 @@ has been removed in Elasticsearch 6.0.0.
==== Analyze API changes

The deprecated request parameters and plain text in request body has been removed. Define parameters in request body.

==== Indices exists API

The `ignore_unavailable` and `allow_no_indices` options are no longer accepted
as they could cause undesired results when their values differed from their
defaults.
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,6 @@
}
},
"params": {
"ignore_unavailable": {
"type" : "boolean",
"description" : "Whether specified concrete indices should be ignored when unavailable (missing or closed)"
},
"allow_no_indices": {
"type" : "boolean",
"description" : "Whether to ignore if a wildcard indices expression resolves into no concrete indices. (This includes `_all` string or when no indices have been specified)"
},
"expand_wildcards": {
"type" : "enum",
"options" : ["open","closed","none","all"],
Expand Down

0 comments on commit 7120587

Please sign in to comment.