ignore_unavailable shouldn't ignore closed indices#9047
ignore_unavailable shouldn't ignore closed indices#9047martijnvg merged 1 commit intoelastic:masterfrom
ignore_unavailable shouldn't ignore closed indices#9047Conversation
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
because the index the alias is pointing to may be in a closed state.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
agreed, lets open another issue for this.
|
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: Is this something that we want to address as well? |
|
@javanna I updated the PR, addressed the minor comments and added tests for the case a single index mentioned doesn't exist. |
|
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 ;) |
9d64cbb to
728c9a8
Compare
…e index is specified in a search or broadcast request. Closes elastic#9047 Closes elastic#7153
c30f55e to
a345e98
Compare
|
This contradicts http://www.elasticsearch.org/guide/en/elasticsearch/reference/current/multi-index.html |
ignore_unavailable shouldn't ignore closed indicesignore_unavailable shouldn't ignore closed indices
…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
The
ignore_unavailablerequest setting shouldn't ignore closed indices if a single index is specified in a search or broadcast request.PR for #7153