Skip to content

Add deprecation warning for Force Merge API #44903

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

Merged
merged 6 commits into from
Aug 6, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,24 @@
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"
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


Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
Expand All @@ -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));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand All @@ -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<String, String> 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");
}
}