`ignore_unavailable` shouldn't ignore closed indices #9047

martijnvg · 2014-12-23T13:50:22Z

The ignore_unavailable request setting shouldn't ignore closed indices if a single index is specified in a search or broadcast request.

PR for #7153

javanna · 2014-12-23T15:54:21Z

src/main/java/org/elasticsearch/cluster/metadata/MetaData.java

     * @param indicesOptions the indices options to be used for the index resolution
     * @return the concrete index obtained as a result of the index resolution
-     * @throws IndexMissingException if the index or alias provided doesn't exist
+     * @throws IndexMissingException                 if the index or alias provided doesn't exist


javanna · 2014-12-23T16:33:52Z

src/main/java/org/elasticsearch/cluster/metadata/MetaData.java

            throw new ElasticsearchIllegalArgumentException("Alias [" + aliasOrIndex + "] has more than one indices associated with it [" + Arrays.toString(indices) + "], can't execute a single index op");
        }

        indexMetaData = this.indices.get(aliasOrIndex);
-        if (indexMetaData != null && indexMetaData.getState() == IndexMetaData.State.CLOSE && failClosed) {
-            throw new IndexClosedException(new Index(aliasOrIndex));
+        if (indexMetaData != null && indexMetaData.getState() == IndexMetaData.State.CLOSE) {


can you remind me why we need this check again? Didn't we do it at the beginning of the method already? Maybe this if was a leftover?

because the index the alias is pointing to may be in a closed state.

I think we can remove this if completely, cause we call the same method given the same input at the beginning of the method, this condition will never be true here.

Actually, I did some digging, this if was introduced with #6475, and I think its purpose was to check that state of indices after aliases resolution. This is a bug that we should address on a separate issue, that should be about index state checks when resolving aliases, since it seems we don't check that at all at this time.

agreed, lets open another issue for this.

javanna · 2014-12-23T16:42:53Z

I left a couple of minor comments @martijnvg . The one thing I am not sure about is that the PR addresses the case of closed indices, while the related issue might be referring to more usecases around the single index special case, like:

curl -XGET localhost:9200/index1/_search?ignore_unavailable=true

# fails if index1 is not there, although we said ignore_unavailable=true
{
  "error" : "IndexMissingException[[index1] missing]",
  "status" : 404
}

Is this something that we want to address as well?

martijnvg · 2014-12-23T17:34:58Z

@javanna I updated the PR, addressed the minor comments and added tests for the case a single index mentioned doesn't exist.

javanna · 2014-12-23T17:36:01Z

Thanks @mvg LGTM, the single missing index case I mentioned before was something I overlooked on my end, it works as expected already, so this PR addresses what it should addresses, sorry for the confusion ;)

…e index is specified in a search or broadcast request. Closes elastic#9047 Closes elastic#7153

…e index is specified in a search or broadcast request. Closes #9047 Closes #7153

zde · 2015-02-12T09:58:07Z

This contradicts http://www.elasticsearch.org/guide/en/elasticsearch/reference/current/multi-index.html
I see no rationale for special handling of the single non-wildcard index case.

…e index is specified in a search or broadcast request. Closes elastic#9047 Closes elastic#7153

This is a followup to elastic#21689 where we removed a misplaced try catch for IndexMissingException and IndexClosedException which was related to elastic#9047 (at least for the index closed case). The code block within the change was moved as part of elastic#20890, which made the catch redundant. It was somehow used before (e.g. in 5.0) but it doesn't seem that this catch had any effect. Added tests to verify that. In fact a specific catch added to the search api only would defeat the purpose of having common indices options that work throughout all our APIs. Relates to elastic#21689

This is a followup to #21689 where we removed a misplaced try catch for IndexMissingException and IndexClosedException which was related to #9047 (at least for the index closed case). The code block within the change was moved as part of #20890, which made the catch redundant. It was somehow used before (e.g. in 5.0) but it doesn't seem that this catch had any effect. Added tests to verify that. In fact a specific catch added to the search api only would defeat the purpose of having common indices options that work throughout all our APIs. Relates to #21689

martijnvg added >bug v1.4.3 v1.5.0 v2.0.0-beta1 labels Dec 23, 2014

martijnvg mentioned this pull request Dec 23, 2014

ignore_unavailable=true ignored when a single index is specified #7153

Closed

javanna reviewed Dec 23, 2014
View reviewed changes

martijnvg added the review label Dec 23, 2014

javanna reviewed Dec 23, 2014
View reviewed changes

martijnvg force-pushed the ignore_missing_closed_indices branch from 9d64cbb to 728c9a8 Compare December 23, 2014 18:12

Core: ignore_unavailable shouldn't ignore closed indices if a singl…

a345e98

…e index is specified in a search or broadcast request. Closes elastic#9047 Closes elastic#7153

martijnvg force-pushed the ignore_missing_closed_indices branch from c30f55e to a345e98 Compare December 24, 2014 09:46

martijnvg added a commit that referenced this pull request Dec 24, 2014

Core: ignore_unavailable shouldn't ignore closed indices if a singl…

cce5883

…e index is specified in a search or broadcast request. Closes #9047 Closes #7153

martijnvg added a commit that referenced this pull request Dec 24, 2014

Core: ignore_unavailable shouldn't ignore closed indices if a singl…

c167242

…e index is specified in a search or broadcast request. Closes #9047 Closes #7153

martijnvg merged commit a345e98 into elastic:master Dec 24, 2014

clintongormley added :Core/Infra/Core Core issues without another label and removed review labels Mar 19, 2015

martijnvg deleted the ignore_missing_closed_indices branch May 18, 2015 23:28

clintongormley changed the title ~~Core: ignore_unavailable shouldn't ignore closed indices~~ ignore_unavailable shouldn't ignore closed indices Jun 7, 2015

mute pushed a commit to mute/elasticsearch that referenced this pull request Jul 29, 2015

Core: ignore_unavailable shouldn't ignore closed indices if a singl…

01ba747

…e index is specified in a search or broadcast request. Closes elastic#9047 Closes elastic#7153

javanna mentioned this pull request Nov 21, 2016

Add indices options tests to search api REST tests #21701

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`ignore_unavailable` shouldn't ignore closed indices #9047

`ignore_unavailable` shouldn't ignore closed indices #9047

martijnvg commented Dec 23, 2014

javanna Dec 23, 2014

javanna Dec 23, 2014

martijnvg Dec 23, 2014

javanna Dec 23, 2014

javanna Dec 23, 2014

martijnvg Dec 23, 2014

javanna commented Dec 23, 2014

martijnvg commented Dec 23, 2014

javanna commented Dec 23, 2014

zde commented Feb 12, 2015

ignore_unavailable shouldn't ignore closed indices #9047

ignore_unavailable shouldn't ignore closed indices #9047

Conversation

martijnvg commented Dec 23, 2014

javanna Dec 23, 2014

Choose a reason for hiding this comment

javanna Dec 23, 2014

Choose a reason for hiding this comment

martijnvg Dec 23, 2014

Choose a reason for hiding this comment

javanna Dec 23, 2014

Choose a reason for hiding this comment

javanna Dec 23, 2014

Choose a reason for hiding this comment

martijnvg Dec 23, 2014

Choose a reason for hiding this comment

javanna commented Dec 23, 2014

martijnvg commented Dec 23, 2014

javanna commented Dec 23, 2014

zde commented Feb 12, 2015

`ignore_unavailable` shouldn't ignore closed indices #9047

`ignore_unavailable` shouldn't ignore closed indices #9047