-
Notifications
You must be signed in to change notification settings - Fork 25.6k
ignore_unavailable shouldn't ignore closed indices
#9047
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -679,7 +679,7 @@ public String[] concreteIndices(IndicesOptions indicesOptions, String... aliases | |
|
|
||
| // optimize for single element index (common case) | ||
| if (aliasesOrIndices.length == 1) { | ||
| return concreteIndices(aliasesOrIndices[0], indicesOptions.allowNoIndices(), failClosed, indicesOptions.allowAliasesToMultipleIndices()); | ||
| return concreteIndices(aliasesOrIndices[0], indicesOptions.allowNoIndices(), indicesOptions); | ||
| } | ||
|
|
||
| // check if its a possible aliased index, if not, just return the passed array | ||
|
|
@@ -712,7 +712,7 @@ public String[] concreteIndices(IndicesOptions indicesOptions, String... aliases | |
|
|
||
| Set<String> actualIndices = new HashSet<>(); | ||
| for (String aliasOrIndex : aliasesOrIndices) { | ||
| String[] indices = concreteIndices(aliasOrIndex, indicesOptions.ignoreUnavailable(), failClosed, indicesOptions.allowAliasesToMultipleIndices()); | ||
| String[] indices = concreteIndices(aliasOrIndex, indicesOptions.ignoreUnavailable(), indicesOptions); | ||
| Collections.addAll(actualIndices, indices); | ||
| } | ||
|
|
||
|
|
@@ -723,16 +723,15 @@ public String[] concreteIndices(IndicesOptions indicesOptions, String... aliases | |
| } | ||
|
|
||
| /** | ||
| * | ||
| * Utility method that allows to resolve an index or alias to its corresponding single concrete index. | ||
| * Callers should make sure they provide proper {@link org.elasticsearch.action.support.IndicesOptions} | ||
| * that require a single index as a result. The indices resolution must in fact return a single index when | ||
| * using this method, an {@link org.elasticsearch.ElasticsearchIllegalArgumentException} gets thrown otherwise. | ||
| * | ||
| * @param indexOrAlias the index or alias to be resolved to concrete index | ||
| * @param indexOrAlias the index or alias to be resolved to concrete index | ||
| * @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 | ||
| * @throws ElasticsearchIllegalArgumentException if the index resolution lead to more than one index | ||
| */ | ||
| public String concreteSingleIndex(String indexOrAlias, IndicesOptions indicesOptions) throws IndexMissingException, ElasticsearchIllegalArgumentException { | ||
|
|
@@ -743,28 +742,40 @@ public String concreteSingleIndex(String indexOrAlias, IndicesOptions indicesOpt | |
| return indices[0]; | ||
| } | ||
|
|
||
| private String[] concreteIndices(String aliasOrIndex, boolean allowNoIndices, boolean failClosed, boolean allowMultipleIndices) throws IndexMissingException, ElasticsearchIllegalArgumentException { | ||
| private String[] concreteIndices(String aliasOrIndex, boolean allowNoIndices, IndicesOptions options) throws IndexMissingException, ElasticsearchIllegalArgumentException { | ||
| boolean failClosed = options.forbidClosedIndices() && !options.ignoreUnavailable(); | ||
|
|
||
| // a quick check, if this is an actual index, if so, return it | ||
| IndexMetaData indexMetaData = indices.get(aliasOrIndex); | ||
| if (indexMetaData != null) { | ||
| if (indexMetaData.getState() == IndexMetaData.State.CLOSE && failClosed) { | ||
| throw new IndexClosedException(new Index(aliasOrIndex)); | ||
| if (indexMetaData.getState() == IndexMetaData.State.CLOSE) { | ||
| if (failClosed) { | ||
| throw new IndexClosedException(new Index(aliasOrIndex)); | ||
| } else { | ||
| return options.forbidClosedIndices() ? Strings.EMPTY_ARRAY : new String[]{aliasOrIndex}; | ||
| } | ||
| } else { | ||
| return new String[]{aliasOrIndex}; | ||
| return new String[]{aliasOrIndex}; | ||
| } | ||
| } | ||
| // not an actual index, fetch from an alias | ||
| String[] indices = aliasAndIndexToIndexMap.getOrDefault(aliasOrIndex, Strings.EMPTY_ARRAY); | ||
| if (indices.length == 0 && !allowNoIndices) { | ||
| throw new IndexMissingException(new Index(aliasOrIndex)); | ||
| } | ||
| if (indices.length > 1 && !allowMultipleIndices) { | ||
| if (indices.length > 1 && !options.allowAliasesToMultipleIndices()) { | ||
| 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) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can you remind me why we need this check again? Didn't we do it at the beginning of the method already? Maybe this
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. because the index the alias is pointing to may be in a closed state.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we can remove this
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. agreed, lets open another issue for this. |
||
| if (failClosed) { | ||
| throw new IndexClosedException(new Index(aliasOrIndex)); | ||
| } else { | ||
| if (options.forbidClosedIndices()) { | ||
| return Strings.EMPTY_ARRAY; | ||
| } | ||
| } | ||
| } | ||
| return indices; | ||
| } | ||
|
|
@@ -1317,7 +1328,7 @@ public static void toXContent(MetaData metaData, XContentBuilder builder, ToXCon | |
|
|
||
| for (ObjectObjectCursor<String, Custom> cursor : metaData.customs()) { | ||
| Custom.Factory factory = lookupFactorySafe(cursor.key); | ||
| if(factory.context().contains(context)) { | ||
| if (factory.context().contains(context)) { | ||
| builder.startObject(cursor.key); | ||
| factory.toXContent(cursor.value, builder, params); | ||
| builder.endObject(); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why?