From a92873d3641bbc63913986a9c464f21bb6f7a873 Mon Sep 17 00:00:00 2001 From: Tanguy Leroux Date: Fri, 26 Jul 2019 15:34:24 +0200 Subject: [PATCH 1/4] Add deprecation warning for Force Merge API --- .../test/indices.forcemerge/10_basic.yml | 23 ++++++++++++++ .../admin/indices/RestForceMergeAction.java | 8 +++++ .../forcemerge/RestForceMergeActionTests.java | 30 +++++++++++++++++-- 3 files changed, 59 insertions(+), 2 deletions(-) diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.forcemerge/10_basic.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.forcemerge/10_basic.yml index 6f1c6ea949665..cf033c664f25e 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.forcemerge/10_basic.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.forcemerge/10_basic.yml @@ -8,3 +8,26 @@ indices.forcemerge: index: testing max_num_segments: 1 + +--- +"Check deprecation warning when incompatible only_expunge_deletes and max_num_segments values are both set": + - skip: + version: "7.3.99 - 7.9.99" + reason: "deprecation warning about only_expunge_deletes and max_num_segments added in 7.4" + features: "warnings" + + - do: + indices.create: + index: test + + - do: + warnings: + - 'setting only_expunge_deletes and max_num_segments at the same time is deprecated and will be rejected in a future version' + indices.forcemerge: + index: test + max_num_segments: 10 + only_expunge_deletes: true + + - match: { acknowledged: true } + + diff --git a/server/src/main/java/org/elasticsearch/rest/action/admin/indices/RestForceMergeAction.java b/server/src/main/java/org/elasticsearch/rest/action/admin/indices/RestForceMergeAction.java index 2d9d691d2c71a..13a826300b599 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/admin/indices/RestForceMergeAction.java +++ b/server/src/main/java/org/elasticsearch/rest/action/admin/indices/RestForceMergeAction.java @@ -19,10 +19,12 @@ package org.elasticsearch.rest.action.admin.indices; +import org.apache.logging.log4j.LogManager; import org.elasticsearch.action.admin.indices.forcemerge.ForceMergeRequest; import org.elasticsearch.action.support.IndicesOptions; import org.elasticsearch.client.node.NodeClient; import org.elasticsearch.common.Strings; +import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.rest.BaseRestHandler; import org.elasticsearch.rest.RestController; @@ -35,6 +37,8 @@ public class RestForceMergeAction extends BaseRestHandler { + private static final DeprecationLogger deprecationLogger = new DeprecationLogger(LogManager.getLogger(RestForceMergeAction.class)); + public RestForceMergeAction(final Settings settings, final RestController controller) { super(settings); controller.registerHandler(POST, "/_forcemerge", this); @@ -53,6 +57,10 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC mergeRequest.maxNumSegments(request.paramAsInt("max_num_segments", mergeRequest.maxNumSegments())); mergeRequest.onlyExpungeDeletes(request.paramAsBoolean("only_expunge_deletes", mergeRequest.onlyExpungeDeletes())); mergeRequest.flush(request.paramAsBoolean("flush", mergeRequest.flush())); + if (mergeRequest.onlyExpungeDeletes() && mergeRequest.maxNumSegments() != ForceMergeRequest.Defaults.MAX_NUM_SEGMENTS) { + deprecationLogger.deprecatedAndMaybeLog("force_merge_expunge_deletes_and_max_num_segments_deprecation", + "setting only_expunge_deletes and max_num_segments at the same time is deprecated and will be rejected in a future version"); + } return channel -> client.admin().indices().forceMerge(mergeRequest, new RestToXContentListener<>(channel)); } diff --git a/server/src/test/java/org/elasticsearch/action/admin/indices/forcemerge/RestForceMergeActionTests.java b/server/src/test/java/org/elasticsearch/action/admin/indices/forcemerge/RestForceMergeActionTests.java index 2d4093d8525d9..c4a4169ba1623 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/indices/forcemerge/RestForceMergeActionTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/indices/forcemerge/RestForceMergeActionTests.java @@ -26,15 +26,25 @@ import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.common.xcontent.json.JsonXContent; import org.elasticsearch.rest.RestController; +import org.elasticsearch.rest.RestRequest; import org.elasticsearch.rest.action.admin.indices.RestForceMergeAction; -import org.elasticsearch.test.ESTestCase; import org.elasticsearch.test.rest.FakeRestChannel; import org.elasticsearch.test.rest.FakeRestRequest; +import org.elasticsearch.test.rest.RestActionTestCase; +import org.junit.Before; + +import java.util.HashMap; +import java.util.Map; import static org.hamcrest.Matchers.equalTo; import static org.mockito.Mockito.mock; -public class RestForceMergeActionTests extends ESTestCase { +public class RestForceMergeActionTests extends RestActionTestCase { + + @Before + public void setUpAction() { + new RestForceMergeAction(Settings.EMPTY, controller()); + } public void testBodyRejection() throws Exception { final RestForceMergeAction handler = new RestForceMergeAction(Settings.EMPTY, mock(RestController.class)); @@ -48,4 +58,20 @@ public void testBodyRejection() throws Exception { assertThat(e.getMessage(), equalTo("request [GET /_forcemerge] does not support having a body")); } + public void testDeprecationMessage() { + final Map params = new HashMap<>(); + params.put("only_expunge_deletes", Boolean.TRUE.toString()); + params.put("max_num_segments", Integer.toString(randomIntBetween(0, 10))); + params.put("flush", Boolean.toString(randomBoolean())); + + final RestRequest request = new FakeRestRequest.Builder(NamedXContentRegistry.EMPTY) + .withPath("/_forcemerge") + .withMethod(RestRequest.Method.POST) + .withParams(params) + .build(); + + dispatchRequest(request); + assertWarnings("setting only_expunge_deletes and max_num_segments at the same time is deprecated " + + "and will be rejected in a future version"); + } } From ebf213dbfe82162f7a33560e93283056c8a89ab1 Mon Sep 17 00:00:00 2001 From: Tanguy Leroux Date: Fri, 26 Jul 2019 17:33:39 +0200 Subject: [PATCH 2/4] fix tests --- .../client/documentation/IndicesClientDocumentationIT.java | 4 ++++ .../rest-api-spec/test/indices.forcemerge/10_basic.yml | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/IndicesClientDocumentationIT.java b/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/IndicesClientDocumentationIT.java index fa25ee52b4bad..ae13d8c2536fc 100644 --- a/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/IndicesClientDocumentationIT.java +++ b/client/rest-high-level/src/test/java/org/elasticsearch/client/documentation/IndicesClientDocumentationIT.java @@ -1313,6 +1313,10 @@ public void testForceMergeIndex() throws Exception { request.onlyExpungeDeletes(true); // <1> // end::force-merge-request-only-expunge-deletes + // set only expunge deletes back to its default value + // as it is mutually exclusive with max. num. segments + request.onlyExpungeDeletes(ForceMergeRequest.Defaults.ONLY_EXPUNGE_DELETES); + // tag::force-merge-request-flush request.flush(true); // <1> // end::force-merge-request-flush diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.forcemerge/10_basic.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.forcemerge/10_basic.yml index cf033c664f25e..b07c8f90b62e8 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.forcemerge/10_basic.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.forcemerge/10_basic.yml @@ -12,7 +12,7 @@ --- "Check deprecation warning when incompatible only_expunge_deletes and max_num_segments values are both set": - skip: - version: "7.3.99 - 7.9.99" + version: "7.4.0 - 7.9.99" reason: "deprecation warning about only_expunge_deletes and max_num_segments added in 7.4" features: "warnings" From 6d8e78125181fc361fc19de07e1ebc088cc33042 Mon Sep 17 00:00:00 2001 From: Tanguy Leroux Date: Sat, 27 Jul 2019 15:31:24 +0200 Subject: [PATCH 3/4] Fix test? --- .../rest-api-spec/test/indices.forcemerge/10_basic.yml | 2 -- 1 file changed, 2 deletions(-) diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.forcemerge/10_basic.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.forcemerge/10_basic.yml index b07c8f90b62e8..40844b3fdeebe 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.forcemerge/10_basic.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.forcemerge/10_basic.yml @@ -28,6 +28,4 @@ max_num_segments: 10 only_expunge_deletes: true - - match: { acknowledged: true } - From 203b6156bd7f4a9215886ebda41e06f2c593a4f1 Mon Sep 17 00:00:00 2001 From: Tanguy Leroux Date: Mon, 5 Aug 2019 12:59:23 +0200 Subject: [PATCH 4/4] Fix version range --- .../rest-api-spec/test/indices.forcemerge/10_basic.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.forcemerge/10_basic.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.forcemerge/10_basic.yml index 40844b3fdeebe..c71a2e5e43429 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.forcemerge/10_basic.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.forcemerge/10_basic.yml @@ -12,7 +12,7 @@ --- "Check deprecation warning when incompatible only_expunge_deletes and max_num_segments values are both set": - skip: - version: "7.4.0 - 7.9.99" + version: " - 7.3.99" reason: "deprecation warning about only_expunge_deletes and max_num_segments added in 7.4" features: "warnings"