From 2131e119d7a29e099e0307125d58ba960744f072 Mon Sep 17 00:00:00 2001 From: Simon Willnauer Date: Wed, 7 Nov 2018 18:43:27 +0100 Subject: [PATCH] Apply `ignore_throttled` also to concrete indices (#35335) Today we only apply `ingore_throttled` to expansions from wildcards, date math expressions and aliases. Yet, this is tricky since we might have resolved certain expressions in pre-filter steps like security. It's more consistent to apply this logic to all expressions including concrete indices. Relates to #34354 --- .../action/support/IndicesOptions.java | 2 +- .../metadata/IndexNameExpressionResolver.java | 16 +++++----------- .../IndexNameExpressionResolverTests.java | 6 ++---- .../elasticsearch/search/SearchServiceTests.java | 3 ++- 4 files changed, 10 insertions(+), 17 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/support/IndicesOptions.java b/server/src/main/java/org/elasticsearch/action/support/IndicesOptions.java index 9753a6f712b..c38c2a1c69a 100644 --- a/server/src/main/java/org/elasticsearch/action/support/IndicesOptions.java +++ b/server/src/main/java/org/elasticsearch/action/support/IndicesOptions.java @@ -248,7 +248,7 @@ public class IndicesOptions implements ToXContentFragment { /** * - * @return whether indices that are marked as throttled should be ignored when resolving a wildcard or alias + * @return whether indices that are marked as throttled should be ignored */ public boolean ignoreThrottled() { return options.contains(Option.IGNORE_THROTTLED); diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java b/server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java index 1e7658b1054..9baedf6b65e 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java @@ -147,7 +147,6 @@ public class IndexNameExpressionResolver { if (indexExpressions == null || indexExpressions.length == 0) { indexExpressions = new String[]{MetaData.ALL}; } - Set originalIndexExpression = Sets.newHashSet(indexExpressions); MetaData metaData = context.getState().metaData(); IndicesOptions options = context.getOptions(); final boolean failClosed = options.forbidClosedIndices() && options.ignoreUnavailable() == false; @@ -197,7 +196,7 @@ public class IndexNameExpressionResolver { " The write index may be explicitly disabled using is_write_index=false or the alias points to multiple" + " indices without one being designated as a write index"); } - if (addIndex(writeIndex, context, originalIndexExpression)) { + if (addIndex(writeIndex, context)) { concreteIndices.add(writeIndex.getIndex()); } } else { @@ -216,12 +215,12 @@ public class IndexNameExpressionResolver { if (failClosed) { throw new IndexClosedException(index.getIndex()); } else { - if (options.forbidClosedIndices() == false && addIndex(index, context, originalIndexExpression)) { + if (options.forbidClosedIndices() == false && addIndex(index, context)) { concreteIndices.add(index.getIndex()); } } } else if (index.getState() == IndexMetaData.State.OPEN) { - if (addIndex(index, context, originalIndexExpression)) { + if (addIndex(index, context)) { concreteIndices.add(index.getIndex()); } } else { @@ -239,13 +238,8 @@ public class IndexNameExpressionResolver { return concreteIndices.toArray(new Index[concreteIndices.size()]); } - private static boolean addIndex(IndexMetaData metaData, Context context, Set originalIndices) { - if (context.options.ignoreThrottled()) { - if (originalIndices.contains(metaData.getIndex().getName()) == false) { - return IndexSettings.INDEX_SEARCH_THROTTLED.get(metaData.getSettings()) == false; - } - } - return true; + private static boolean addIndex(IndexMetaData metaData, Context context) { + return (context.options.ignoreThrottled() && IndexSettings.INDEX_SEARCH_THROTTLED.get(metaData.getSettings())) == false; } private static IllegalArgumentException aliasesNotSupportedException(String expression) { diff --git a/server/src/test/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolverTests.java b/server/src/test/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolverTests.java index ab5756fccfc..5c96f6f1cbc 100644 --- a/server/src/test/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolverTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolverTests.java @@ -1357,10 +1357,9 @@ public class IndexNameExpressionResolverTests extends ESTestCase { { Index[] indices = indexNameExpressionResolver.concreteIndices(state, IndicesOptions.STRICT_EXPAND_OPEN_FORBID_CLOSED_IGNORE_THROTTLED, "ind*", "test-index"); - assertEquals(2, indices.length); + assertEquals(1, indices.length); Arrays.sort(indices, Comparator.comparing(Index::getName)); assertEquals("index", indices[0].getName()); - assertEquals("test-index", indices[1].getName()); } { @@ -1368,10 +1367,9 @@ public class IndexNameExpressionResolverTests extends ESTestCase { new IndicesOptions(EnumSet.of(IndicesOptions.Option.ALLOW_NO_INDICES, IndicesOptions.Option.IGNORE_THROTTLED), EnumSet.of(IndicesOptions.WildcardStates.OPEN)), "ind*", "test-index"); - assertEquals(2, indices.length); + assertEquals(1, indices.length); Arrays.sort(indices, Comparator.comparing(Index::getName)); assertEquals("index", indices[0].getName()); - assertEquals("test-index", indices[1].getName()); } { Index[] indices = indexNameExpressionResolver.concreteIndices(state, diff --git a/server/src/test/java/org/elasticsearch/search/SearchServiceTests.java b/server/src/test/java/org/elasticsearch/search/SearchServiceTests.java index 37fc9cac029..50f654f4f49 100644 --- a/server/src/test/java/org/elasticsearch/search/SearchServiceTests.java +++ b/server/src/test/java/org/elasticsearch/search/SearchServiceTests.java @@ -533,7 +533,8 @@ public class SearchServiceTests extends ESSingleNodeTestCase { Index index = resolveIndex("throttled_threadpool_index"); assertTrue(service.getIndicesService().indexServiceSafe(index).getIndexSettings().isSearchThrottled()); client().prepareIndex("throttled_threadpool_index", "_doc", "1").setSource("field", "value").setRefreshPolicy(IMMEDIATE).get(); - SearchResponse searchResponse = client().prepareSearch("throttled_threadpool_index").setSize(1).get(); + SearchResponse searchResponse = client().prepareSearch("throttled_threadpool_index") + .setIndicesOptions(IndicesOptions.STRICT_EXPAND_OPEN_FORBID_CLOSED).setSize(1).get(); assertSearchHits(searchResponse, "1"); // we add a search action listener in a plugin above to assert that this is actually used client().execute(