From 6af1cb84d6f191f4fe02f4fe0ff22d0891e2245d Mon Sep 17 00:00:00 2001 From: Martijn van Groningen Date: Wed, 22 Sep 2021 14:53:49 +0200 Subject: [PATCH 1/2] Warn when searching a frozen index. The coordinating node should warn when a search request is targeting a frozen index. Relates to #70192 --- .../action/search/TransportSearchAction.java | 25 +++++++- .../search/SearchServiceTests.java | 9 ++- x-pack/plugin/build.gradle | 4 ++ .../index/engine/frozen/FrozenIndexTests.java | 57 +++++++++++-------- ...stractSearchableSnapshotsRestTestCase.java | 29 +++++++--- .../test/indices.freeze/10_basic.yml | 3 + .../xpack/restart/FullClusterRestartIT.java | 4 +- 7 files changed, 92 insertions(+), 39 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/search/TransportSearchAction.java b/server/src/main/java/org/elasticsearch/action/search/TransportSearchAction.java index 77da80ad0f7e0..e3c9608c808f9 100644 --- a/server/src/main/java/org/elasticsearch/action/search/TransportSearchAction.java +++ b/server/src/main/java/org/elasticsearch/action/search/TransportSearchAction.java @@ -20,6 +20,7 @@ import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.block.ClusterBlockException; import org.elasticsearch.cluster.block.ClusterBlockLevel; +import org.elasticsearch.cluster.metadata.IndexMetadata; import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; import org.elasticsearch.cluster.node.DiscoveryNode; import org.elasticsearch.cluster.node.DiscoveryNodes; @@ -33,6 +34,8 @@ import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.io.stream.NamedWriteableRegistry; import org.elasticsearch.common.io.stream.Writeable; +import org.elasticsearch.common.logging.DeprecationCategory; +import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.common.settings.Setting; import org.elasticsearch.common.settings.Setting.Property; import org.elasticsearch.common.util.concurrent.AtomicArray; @@ -92,6 +95,10 @@ public class TransportSearchAction extends HandledTransportAction { + private static final DeprecationLogger DEPRECATION_LOGGER = DeprecationLogger.getLogger(TransportSearchAction.class); + public static final String FROZEN_INDICES_DEPRECATION_MESSAGE = "Searching frozen indices [{}] is deprecated." + + " Consider cold or frozen tiers in place of frozen indices. The frozen feature will be removed in a feature release."; + /** The maximum number of shards for a single search request. */ public static final Setting SHARD_COUNT_LIMIT_SETTING = Setting.longSetting( "action.search.shard_count.limit", Long.MAX_VALUE, 1L, Property.Dynamic, Property.NodeScope); @@ -592,7 +599,23 @@ private Index[] resolveLocalIndices(OriginalIndices localIndices, if (localIndices == null) { return Index.EMPTY_ARRAY; //don't search on any local index (happens when only remote indices were specified) } - return indexNameExpressionResolver.concreteIndices(clusterState, localIndices, timeProvider.getAbsoluteStartMillis()); + + List frozenIndices = null; + Index[] indices = indexNameExpressionResolver.concreteIndices(clusterState, localIndices, timeProvider.getAbsoluteStartMillis()); + for (Index index : indices) { + IndexMetadata indexMetadata = clusterState.metadata().index(index); + if (indexMetadata.getSettings().getAsBoolean("index.frozen", false)) { + if (frozenIndices == null) { + frozenIndices = new ArrayList<>(); + } + frozenIndices.add(index.getName()); + } + } + if (frozenIndices != null) { + DEPRECATION_LOGGER.critical(DeprecationCategory.INDICES, "search-frozen-indices", FROZEN_INDICES_DEPRECATION_MESSAGE, + String.join(",", frozenIndices)); + } + return indices; } private void executeSearch(SearchTask task, SearchTimeProvider timeProvider, SearchRequest searchRequest, diff --git a/server/src/test/java/org/elasticsearch/search/SearchServiceTests.java b/server/src/test/java/org/elasticsearch/search/SearchServiceTests.java index 506c6845991dc..ca403ddfd1829 100644 --- a/server/src/test/java/org/elasticsearch/search/SearchServiceTests.java +++ b/server/src/test/java/org/elasticsearch/search/SearchServiceTests.java @@ -26,6 +26,7 @@ import org.elasticsearch.action.search.SearchScrollRequest; import org.elasticsearch.action.search.SearchShardTask; import org.elasticsearch.action.search.SearchType; +import org.elasticsearch.action.search.TransportSearchAction; import org.elasticsearch.action.support.IndicesOptions; import org.elasticsearch.action.support.PlainActionFuture; import org.elasticsearch.action.support.WriteRequest; @@ -917,16 +918,18 @@ public void testExpandSearchThrottled() { } public void testExpandSearchFrozen() { - createIndex("frozen_index"); + String indexName = "frozen_index"; + createIndex(indexName); client().execute( InternalOrPrivateSettingsPlugin.UpdateInternalOrPrivateAction.INSTANCE, - new InternalOrPrivateSettingsPlugin.UpdateInternalOrPrivateAction.Request("frozen_index", + new InternalOrPrivateSettingsPlugin.UpdateInternalOrPrivateAction.Request(indexName, "index.frozen", "true")) .actionGet(); - client().prepareIndex("frozen_index").setId("1").setSource("field", "value").setRefreshPolicy(IMMEDIATE).get(); + client().prepareIndex(indexName).setId("1").setSource("field", "value").setRefreshPolicy(IMMEDIATE).get(); assertHitCount(client().prepareSearch().get(), 0L); assertHitCount(client().prepareSearch().setIndicesOptions(IndicesOptions.STRICT_EXPAND_OPEN_FORBID_CLOSED).get(), 1L); + assertWarnings(TransportSearchAction.FROZEN_INDICES_DEPRECATION_MESSAGE.replace("{}", indexName)); } public void testCreateReduceContext() { diff --git a/x-pack/plugin/build.gradle b/x-pack/plugin/build.gradle index ada3688e85aa0..4e2545397919d 100644 --- a/x-pack/plugin/build.gradle +++ b/x-pack/plugin/build.gradle @@ -166,6 +166,10 @@ tasks.named("yamlRestTestV7CompatTransform").configure({ task -> }) tasks.named("yamlRestTestV7CompatTest").configure { systemProperty 'tests.rest.blacklist', [ + // UNMUTE after #78184 is backported to 7.x + 'indices.freeze/10_basic/Basic', + 'indices.freeze/10_basic/Test index options', + '', // to support it, it would require to almost revert back the #48725 and complicate the code 'vectors/10_dense_vector_basic/Deprecated function signature', // not going to be supported diff --git a/x-pack/plugin/frozen-indices/src/internalClusterTest/java/org/elasticsearch/index/engine/frozen/FrozenIndexTests.java b/x-pack/plugin/frozen-indices/src/internalClusterTest/java/org/elasticsearch/index/engine/frozen/FrozenIndexTests.java index 85f1cb26056f3..7ff2c22e34ec3 100644 --- a/x-pack/plugin/frozen-indices/src/internalClusterTest/java/org/elasticsearch/index/engine/frozen/FrozenIndexTests.java +++ b/x-pack/plugin/frozen-indices/src/internalClusterTest/java/org/elasticsearch/index/engine/frozen/FrozenIndexTests.java @@ -20,6 +20,7 @@ import org.elasticsearch.action.search.SearchRequest; import org.elasticsearch.action.search.SearchResponse; import org.elasticsearch.action.search.SearchType; +import org.elasticsearch.action.search.TransportSearchAction; import org.elasticsearch.action.support.IndicesOptions; import org.elasticsearch.action.support.PlainActionFuture; import org.elasticsearch.cluster.block.ClusterBlockException; @@ -92,17 +93,18 @@ String openReaders(TimeValue keepAlive, String... indices) { } public void testCloseFreezeAndOpen() throws Exception { - createIndex("index", Settings.builder().put("index.number_of_shards", 2).build()); - client().prepareIndex("index").setId("1").setSource("field", "value").setRefreshPolicy(IMMEDIATE).get(); - client().prepareIndex("index").setId("2").setSource("field", "value").setRefreshPolicy(IMMEDIATE).get(); - client().prepareIndex("index").setId("3").setSource("field", "value").setRefreshPolicy(IMMEDIATE).get(); - assertAcked(client().execute(FreezeIndexAction.INSTANCE, new FreezeRequest("index")).actionGet()); + String indexName = "index"; + createIndex(indexName, Settings.builder().put("index.number_of_shards", 2).build()); + client().prepareIndex(indexName).setId("1").setSource("field", "value").setRefreshPolicy(IMMEDIATE).get(); + client().prepareIndex(indexName).setId("2").setSource("field", "value").setRefreshPolicy(IMMEDIATE).get(); + client().prepareIndex(indexName).setId("3").setSource("field", "value").setRefreshPolicy(IMMEDIATE).get(); + assertAcked(client().execute(FreezeIndexAction.INSTANCE, new FreezeRequest(indexName)).actionGet()); expectThrows( ClusterBlockException.class, - () -> client().prepareIndex("index").setId("4").setSource("field", "value").setRefreshPolicy(IMMEDIATE).get() + () -> client().prepareIndex(indexName).setId("4").setSource("field", "value").setRefreshPolicy(IMMEDIATE).get() ); IndicesService indexServices = getInstanceFromNode(IndicesService.class); - Index index = resolveIndex("index"); + Index index = resolveIndex(indexName); IndexService indexService = indexServices.indexServiceSafe(index); IndexShard shard = indexService.getShard(0); Engine engine = IndexShardTestCase.getEngine(shard); @@ -141,7 +143,7 @@ public void testCloseFreezeAndOpen() throws Exception { } while (searchResponse.getHits().getHits().length > 0); client().prepareClearScroll().addScrollId(searchResponse.getScrollId()).get(); - String pitId = openReaders(TimeValue.timeValueMinutes(1), "index"); + String pitId = openReaders(TimeValue.timeValueMinutes(1), indexName); try { for (int from = 0; from < 3; from++) { searchResponse = client().prepareSearch() @@ -160,6 +162,7 @@ public void testCloseFreezeAndOpen() throws Exception { assertFalse(((FrozenEngine) engine).isReaderOpen()); } } + assertWarnings(TransportSearchAction.FROZEN_INDICES_DEPRECATION_MESSAGE.replace("{}", indexName)); } finally { client().execute(ClosePointInTimeAction.INSTANCE, new ClosePointInTimeRequest(pitId)).get(); } @@ -177,11 +180,12 @@ public void testSearchAndGetAPIsAreThrottled() throws IOException { .endObject() .endObject() .endObject(); - createIndex("index", Settings.builder().put("index.number_of_shards", 2).build(), mapping); + String indexName = "index"; + createIndex(indexName, Settings.builder().put("index.number_of_shards", 2).build(), mapping); for (int i = 0; i < 10; i++) { - client().prepareIndex("index").setId("" + i).setSource("field", "foo bar baz").get(); + client().prepareIndex(indexName).setId("" + i).setSource("field", "foo bar baz").get(); } - assertAcked(client().execute(FreezeIndexAction.INSTANCE, new FreezeRequest("index")).actionGet()); + assertAcked(client().execute(FreezeIndexAction.INSTANCE, new FreezeRequest(indexName)).actionGet()); int numRequests = randomIntBetween(20, 50); int numRefreshes = 0; for (int i = 0; i < numRequests; i++) { @@ -190,10 +194,10 @@ public void testSearchAndGetAPIsAreThrottled() throws IOException { // searcher and rewrite the request outside of the search-throttle thread pool switch (randomFrom(Arrays.asList(0, 1, 2))) { case 0: - client().prepareGet("index", "" + randomIntBetween(0, 9)).get(); + client().prepareGet(indexName, "" + randomIntBetween(0, 9)).get(); break; case 1: - client().prepareSearch("index") + client().prepareSearch(indexName) .setIndicesOptions(IndicesOptions.STRICT_EXPAND_OPEN_FORBID_CLOSED) .setSearchType(SearchType.QUERY_THEN_FETCH) .get(); @@ -201,18 +205,19 @@ public void testSearchAndGetAPIsAreThrottled() throws IOException { numRefreshes += 3; break; case 2: - client().prepareTermVectors("index", "" + randomIntBetween(0, 9)).get(); + client().prepareTermVectors(indexName, "" + randomIntBetween(0, 9)).get(); break; case 3: - client().prepareExplain("index", "" + randomIntBetween(0, 9)).setQuery(new MatchAllQueryBuilder()).get(); + client().prepareExplain(indexName, "" + randomIntBetween(0, 9)).setQuery(new MatchAllQueryBuilder()).get(); break; default: assert false; } } - IndicesStatsResponse index = client().admin().indices().prepareStats("index").clear().setRefresh(true).get(); + IndicesStatsResponse index = client().admin().indices().prepareStats(indexName).clear().setRefresh(true).get(); assertEquals(numRefreshes, index.getTotal().refresh.getTotal()); + assertWarnings(TransportSearchAction.FROZEN_INDICES_DEPRECATION_MESSAGE.replace("{}", indexName)); } public void testFreezeAndUnfreeze() { @@ -298,26 +303,28 @@ public void testUnfreezeClosedIndices() { } public void testFreezePattern() { - createIndex("test-idx", Settings.builder().put("index.number_of_shards", 1).build()); - client().prepareIndex("test-idx").setId("1").setSource("field", "value").setRefreshPolicy(IMMEDIATE).get(); + String indexName = "test-idx"; + createIndex(indexName, Settings.builder().put("index.number_of_shards", 1).build()); + client().prepareIndex(indexName).setId("1").setSource("field", "value").setRefreshPolicy(IMMEDIATE).get(); createIndex("test-idx-1", Settings.builder().put("index.number_of_shards", 1).build()); client().prepareIndex("test-idx-1").setId("1").setSource("field", "value").setRefreshPolicy(IMMEDIATE).get(); - assertAcked(client().execute(FreezeIndexAction.INSTANCE, new FreezeRequest("test-idx")).actionGet()); - assertIndexFrozen("test-idx"); + assertAcked(client().execute(FreezeIndexAction.INSTANCE, new FreezeRequest(indexName)).actionGet()); + assertIndexFrozen(indexName); - IndicesStatsResponse index = client().admin().indices().prepareStats("test-idx").clear().setRefresh(true).get(); + IndicesStatsResponse index = client().admin().indices().prepareStats(indexName).clear().setRefresh(true).get(); assertEquals(0, index.getTotal().refresh.getTotal()); - assertHitCount(client().prepareSearch("test-idx").setIndicesOptions(IndicesOptions.STRICT_EXPAND_OPEN_FORBID_CLOSED).get(), 1); - index = client().admin().indices().prepareStats("test-idx").clear().setRefresh(true).get(); + assertHitCount(client().prepareSearch(indexName).setIndicesOptions(IndicesOptions.STRICT_EXPAND_OPEN_FORBID_CLOSED).get(), 1); + index = client().admin().indices().prepareStats(indexName).clear().setRefresh(true).get(); assertEquals(1, index.getTotal().refresh.getTotal()); assertAcked(client().execute(FreezeIndexAction.INSTANCE, new FreezeRequest("test*")).actionGet()); - assertIndexFrozen("test-idx"); + assertIndexFrozen(indexName); assertIndexFrozen("test-idx-1"); - index = client().admin().indices().prepareStats("test-idx").clear().setRefresh(true).get(); + index = client().admin().indices().prepareStats(indexName).clear().setRefresh(true).get(); assertEquals(1, index.getTotal().refresh.getTotal()); index = client().admin().indices().prepareStats("test-idx-1").clear().setRefresh(true).get(); assertEquals(0, index.getTotal().refresh.getTotal()); + assertWarnings(TransportSearchAction.FROZEN_INDICES_DEPRECATION_MESSAGE.replace("{}", indexName)); } public void testCanMatch() throws IOException { diff --git a/x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/xpack/searchablesnapshots/AbstractSearchableSnapshotsRestTestCase.java b/x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/xpack/searchablesnapshots/AbstractSearchableSnapshotsRestTestCase.java index e189c82c2c169..05da22b7ad48b 100644 --- a/x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/xpack/searchablesnapshots/AbstractSearchableSnapshotsRestTestCase.java +++ b/x-pack/plugin/searchable-snapshots/src/test/java/org/elasticsearch/xpack/searchablesnapshots/AbstractSearchableSnapshotsRestTestCase.java @@ -12,6 +12,7 @@ import org.apache.http.client.methods.HttpPut; import org.apache.http.entity.ContentType; import org.apache.http.entity.StringEntity; +import org.elasticsearch.action.search.TransportSearchAction; import org.elasticsearch.client.Request; import org.elasticsearch.client.RequestOptions; import org.elasticsearch.client.Response; @@ -29,6 +30,7 @@ import org.elasticsearch.test.rest.ESRestTestCase; import java.io.IOException; +import java.util.HashSet; import java.util.List; import java.util.Locale; import java.util.Map; @@ -470,19 +472,28 @@ protected static Number count(String index) throws IOException { protected static Map search(String index, QueryBuilder query, Boolean ignoreThrottled) throws IOException { final Request request = new Request(HttpPost.METHOD_NAME, '/' + index + "/_search"); request.setJsonEntity(new SearchSourceBuilder().trackTotalHits(true).query(query).toString()); + + // If warning are returned than these must exist in this set: + Set expectedWarnings = new HashSet<>(); + expectedWarnings.add(TransportSearchAction.FROZEN_INDICES_DEPRECATION_MESSAGE.replace("{}", index)); if (ignoreThrottled != null) { request.addParameter("ignore_throttled", ignoreThrottled.toString()); - RequestOptions requestOptions = RequestOptions.DEFAULT.toBuilder() - .setWarningsHandler( - warnings -> List.of( - "[ignore_throttled] parameter is deprecated because frozen indices have been deprecated. " - + "Consider cold or frozen tiers in place of frozen indices." - ).equals(warnings) == false - ) - .build(); - request.setOptions(requestOptions); + expectedWarnings.add( + "[ignore_throttled] parameter is deprecated because frozen indices have been deprecated. " + + "Consider cold or frozen tiers in place of frozen indices." + ); } + RequestOptions requestOptions = RequestOptions.DEFAULT.toBuilder().setWarningsHandler(warnings -> { + for (String warning : warnings) { + if (expectedWarnings.contains(warning) == false) { + return true; + } + } + return false; + }).build(); + request.setOptions(requestOptions); + final Response response = client().performRequest(request); assertThat( "Failed to execute search request on index [" + index + "]: " + response, diff --git a/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/indices.freeze/10_basic.yml b/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/indices.freeze/10_basic.yml index 9d02de5a95d9c..6d11fff4e81a7 100644 --- a/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/indices.freeze/10_basic.yml +++ b/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/indices.freeze/10_basic.yml @@ -23,6 +23,7 @@ - do: warnings: - "[ignore_throttled] parameter is deprecated because frozen indices have been deprecated. Consider cold or frozen tiers in place of frozen indices." + - "Searching frozen indices [test] is deprecated. Consider cold or frozen tiers in place of frozen indices. The frozen feature will be removed in a feature release." search: rest_total_hits_as_int: true index: test @@ -68,6 +69,7 @@ - do: warnings: - "[ignore_throttled] parameter is deprecated because frozen indices have been deprecated. Consider cold or frozen tiers in place of frozen indices." + - "Searching frozen indices [test,test-01] is deprecated. Consider cold or frozen tiers in place of frozen indices. The frozen feature will be removed in a feature release." search: rest_total_hits_as_int: true index: _all @@ -134,6 +136,7 @@ - do: warnings: - "[ignore_throttled] parameter is deprecated because frozen indices have been deprecated. Consider cold or frozen tiers in place of frozen indices." + - "Searching frozen indices [test] is deprecated. Consider cold or frozen tiers in place of frozen indices. The frozen feature will be removed in a feature release." search: rest_total_hits_as_int: true index: _all diff --git a/x-pack/qa/full-cluster-restart/src/test/java/org/elasticsearch/xpack/restart/FullClusterRestartIT.java b/x-pack/qa/full-cluster-restart/src/test/java/org/elasticsearch/xpack/restart/FullClusterRestartIT.java index abb90d998cc4e..195ba1fa038d2 100644 --- a/x-pack/qa/full-cluster-restart/src/test/java/org/elasticsearch/xpack/restart/FullClusterRestartIT.java +++ b/x-pack/qa/full-cluster-restart/src/test/java/org/elasticsearch/xpack/restart/FullClusterRestartIT.java @@ -739,7 +739,9 @@ public void testFrozenIndexAfterRestarted() throws Exception { assertNoFileBasedRecovery(index, n -> true); final Request request = new Request("GET", "/" + index + "/_search"); request.setOptions(expectWarnings("[ignore_throttled] parameter is deprecated because frozen " + - "indices have been deprecated. Consider cold or frozen tiers in place of frozen indices.")); + "indices have been deprecated. Consider cold or frozen tiers in place of frozen indices.", + "Searching frozen indices [" + index + "] is deprecated. " + + "Consider cold or frozen tiers in place of frozen indices. The frozen feature will be removed in a feature release.")); request.addParameter("ignore_throttled", "false"); assertThat(XContentMapValues.extractValue("hits.total.value", entityAsMap(client().performRequest(request))), equalTo(totalHits)); From 02a7f683e6b99b05dcc770d3bd2635136b9a4706 Mon Sep 17 00:00:00 2001 From: Martijn van Groningen Date: Thu, 23 Sep 2021 14:48:26 +0200 Subject: [PATCH 2/2] removed empty blacklist pattern --- x-pack/plugin/build.gradle | 1 - 1 file changed, 1 deletion(-) diff --git a/x-pack/plugin/build.gradle b/x-pack/plugin/build.gradle index 4e2545397919d..8bbd98da21e3d 100644 --- a/x-pack/plugin/build.gradle +++ b/x-pack/plugin/build.gradle @@ -169,7 +169,6 @@ tasks.named("yamlRestTestV7CompatTest").configure { // UNMUTE after #78184 is backported to 7.x 'indices.freeze/10_basic/Basic', 'indices.freeze/10_basic/Test index options', - '', // to support it, it would require to almost revert back the #48725 and complicate the code 'vectors/10_dense_vector_basic/Deprecated function signature', // not going to be supported