diff --git a/docs/reference/indices/shrink-index.asciidoc b/docs/reference/indices/shrink-index.asciidoc index 81d79c47472df..6af5f94b67659 100644 --- a/docs/reference/indices/shrink-index.asciidoc +++ b/docs/reference/indices/shrink-index.asciidoc @@ -127,7 +127,7 @@ during a shrink operation. With the exception of non-copyable settings, settings from the source index can be copied to the target index by adding the URL parameter `copy_settings=true` to the request. -deprecated[6.4.0, `copy_settings` will default to `true` in 8.x and will be removed in 9.0.0] +deprecated[6.4.0, `copy_settings` will default to `true` in 7.x and will be removed in 8.0.0] [float] === Monitoring the shrink process diff --git a/docs/reference/indices/split-index.asciidoc b/docs/reference/indices/split-index.asciidoc index 58d34cfd9a705..6711cbcdf5bde 100644 --- a/docs/reference/indices/split-index.asciidoc +++ b/docs/reference/indices/split-index.asciidoc @@ -183,7 +183,7 @@ during a split operation. With the exception of non-copyable settings, settings from the source index can be copied to the target index by adding the URL parameter `copy_settings=true` to the request. -deprecated[6.4.0, `copy_settings` will default to `true` in 8.x and will be removed in 9.0.0] +deprecated[6.4.0, `copy_settings` will default to `true` in 7.x and will be removed in 8.0.0] [float] === Monitoring the split process diff --git a/docs/reference/migration/migrate_7_0/api.asciidoc b/docs/reference/migration/migrate_7_0/api.asciidoc index e140fd577bd19..2474f7ad415f2 100644 --- a/docs/reference/migration/migrate_7_0/api.asciidoc +++ b/docs/reference/migration/migrate_7_0/api.asciidoc @@ -65,3 +65,13 @@ deprecated in 6.3.0 and now removed in 7.0.0. In the past, `fields` could be provided either as a parameter, or as part of the request body. Specifying `fields` in the request body as opposed to a parameter was deprecated in 6.4.0, and is now unsupported in 7.0.0. + +[[resize-copy-settings-false]] +==== Copy settings parameter on resize operations no longer accepts `false` + +In 6.4.0 the `copy_settings` parameter was introduced to enable copying the +index settings of the source index on a shrink or split index operation to the +target index. The default value in 6.4.0 was `false`. In accordance with that +setting being immediately deprecated in 6.4.0, `copy_settings` now defaults to +`true` in 7.0.0 and can no longer be set to `false`. The parameter will be +removed in 8.0.0. diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.shrink/30_copy_settings.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.shrink/30_copy_settings.yml index 34757427e6983..b828112d1affe 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.shrink/30_copy_settings.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.shrink/30_copy_settings.yml @@ -64,8 +64,9 @@ - match: { copy-settings-target.settings.index.blocks.write: "true" } - match: { copy-settings-target.settings.index.routing.allocation.include._id: $master } - # now we do a actual shrink and do not copy settings + # now we check that copy_settings can not be set to false - do: + catch: /illegal_argument_exception/ indices.shrink: index: "source" target: "no-copy-settings-target" @@ -79,16 +80,3 @@ warnings: - "parameter [copy_settings] is deprecated but was [false]" - - do: - cluster.health: - wait_for_status: green - - - do: - indices.get_settings: - index: "no-copy-settings-target" - - # only the request setting should be copied - - is_false: no-copy-settings-target.settings.index.merge.scheduler.max_merge_count - - match: { no-copy-settings-target.settings.index.merge.scheduler.max_thread_count: "2" } - - is_false: no-copy-settings-target.settings.index.blocks.write - - is_false: no-copy-settings-target.settings.index.routing.allocation.include._id diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.split/30_copy_settings.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.split/30_copy_settings.yml index 1d3e37aa7b05d..8bb6490ed95b7 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.split/30_copy_settings.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.split/30_copy_settings.yml @@ -67,8 +67,9 @@ - match: { copy-settings-target.settings.index.blocks.write: "true" } - match: { copy-settings-target.settings.index.routing.allocation.include._id: $master } - # now we do a actual shrink and do not copy settings + # now we check that copy_settings can not be set to false - do: + catch: /illegal_argument_exception/ indices.split: index: "source" target: "no-copy-settings-target" @@ -82,17 +83,3 @@ index.merge.scheduler.max_thread_count: 2 warnings: - "parameter [copy_settings] is deprecated but was [false]" - - - do: - cluster.health: - wait_for_status: green - - - do: - indices.get_settings: - index: "no-copy-settings-target" - - # only the request setting should be copied - - is_false: no-copy-settings-target.settings.index.merge.scheduler.max_merge_count - - match: { no-copy-settings-target.settings.index.merge.scheduler.max_thread_count: "2" } - - is_false: no-copy-settings-target.settings.index.blocks.write - - is_false: no-copy-settings-target.settings.index.routing.allocation.include._id diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/shrink/ResizeRequest.java b/server/src/main/java/org/elasticsearch/action/admin/indices/shrink/ResizeRequest.java index f53b5437f03c2..48c99ab26e8ec 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/shrink/ResizeRequest.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/shrink/ResizeRequest.java @@ -56,7 +56,7 @@ public class ResizeRequest extends AcknowledgedRequest implements private CreateIndexRequest targetIndexRequest; private String sourceIndex; private ResizeType type = ResizeType.SHRINK; - private boolean copySettings = false; + private boolean copySettings = true; ResizeRequest() {} @@ -80,6 +80,9 @@ public ActionRequestValidationException validate() { if (type == ResizeType.SPLIT && IndexMetaData.INDEX_NUMBER_OF_SHARDS_SETTING.exists(targetIndexRequest.settings()) == false) { validationException = addValidationError("index.number_of_shards is required for split operations", validationException); } + if (copySettings == false) { + validationException = addValidationError("copySettings can not be set to [false]", validationException); + } return validationException; } diff --git a/server/src/main/java/org/elasticsearch/rest/action/admin/indices/RestResizeHandler.java b/server/src/main/java/org/elasticsearch/rest/action/admin/indices/RestResizeHandler.java index e6c994a85c35d..3f218169436e0 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/admin/indices/RestResizeHandler.java +++ b/server/src/main/java/org/elasticsearch/rest/action/admin/indices/RestResizeHandler.java @@ -57,6 +57,9 @@ public final RestChannelConsumer prepareRequest(final RestRequest request, final copySettings = true; } else { copySettings = Booleans.parseBoolean(rawCopySettings); + if (copySettings == false) { + throw new IllegalArgumentException("copy_settings can not be set to [false]"); + } } } resizeRequest.setCopySettings(copySettings); diff --git a/server/src/test/java/org/elasticsearch/action/admin/indices/create/ShrinkIndexIT.java b/server/src/test/java/org/elasticsearch/action/admin/indices/create/ShrinkIndexIT.java index e48f151081f62..56ffb9d1ea4bd 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/indices/create/ShrinkIndexIT.java +++ b/server/src/test/java/org/elasticsearch/action/admin/indices/create/ShrinkIndexIT.java @@ -443,18 +443,20 @@ public void testCreateShrinkWithIndexSort() throws Exception { // check that index sort cannot be set on the target index IllegalArgumentException exc = expectThrows(IllegalArgumentException.class, () -> client().admin().indices().prepareResizeIndex("source", "target") - .setSettings(Settings.builder() - .put("index.number_of_replicas", 0) - .put("index.number_of_shards", "2") - .put("index.sort.field", "foo") - .build()).get()); + .setSettings(Settings.builder() + .put("index.number_of_replicas", 0) + .put("index.number_of_shards", "2") + .put("index.blocks.write", (String) null) + .put("index.sort.field", "foo") + .build()).get()); assertThat(exc.getMessage(), containsString("can't override index sort when resizing an index")); // check that the index sort order of `source` is correctly applied to the `target` assertAcked(client().admin().indices().prepareResizeIndex("source", "target") - .setSettings(Settings.builder() - .put("index.number_of_replicas", 0) - .put("index.number_of_shards", "2").build()).get()); + .setSettings(Settings.builder() + .put("index.number_of_replicas", 0) + .put("index.number_of_shards", "2") + .put("index.blocks.write", (String) null).build()).get()); ensureGreen(); flushAndRefresh(); GetSettingsResponse settingsResponse = diff --git a/server/src/test/java/org/elasticsearch/action/admin/indices/create/SplitIndexIT.java b/server/src/test/java/org/elasticsearch/action/admin/indices/create/SplitIndexIT.java index fe6e980ab4259..d11e2ee10113c 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/indices/create/SplitIndexIT.java +++ b/server/src/test/java/org/elasticsearch/action/admin/indices/create/SplitIndexIT.java @@ -193,8 +193,9 @@ private void splitToN(int sourceShards, int firstSplitShards, int secondSplitSha .put("index.blocks.write", true)).get(); ensureGreen(); Settings.Builder firstSplitSettingsBuilder = Settings.builder() - .put("index.number_of_replicas", 0) - .put("index.number_of_shards", firstSplitShards); + .put("index.number_of_replicas", 0) + .put("index.number_of_shards", firstSplitShards) + .put("index.blocks.write", (String) null); if (sourceShards == 1 && useRoutingPartition == false && randomBoolean()) { // try to set it if we have a source index with 1 shard firstSplitSettingsBuilder.put("index.number_of_routing_shards", secondSplitShards); } @@ -225,10 +226,11 @@ private void splitToN(int sourceShards, int firstSplitShards, int secondSplitSha ensureGreen(); // now split source into a new index assertAcked(client().admin().indices().prepareResizeIndex("first_split", "second_split") - .setResizeType(ResizeType.SPLIT) - .setSettings(Settings.builder() - .put("index.number_of_replicas", 0) - .put("index.number_of_shards", secondSplitShards).build()).get()); + .setResizeType(ResizeType.SPLIT) + .setSettings(Settings.builder() + .put("index.number_of_replicas", 0) + .put("index.number_of_shards", secondSplitShards) + .put("index.blocks.write", (String) null).build()).get()); ensureGreen(); assertHitCount(client().prepareSearch("second_split").setSize(100).setQuery(new TermsQueryBuilder("foo", "bar")).get(), numDocs); // let it be allocated anywhere and bump replicas @@ -394,10 +396,11 @@ public void testCreateSplitIndex() { final boolean createWithReplicas = randomBoolean(); assertAcked(client().admin().indices().prepareResizeIndex("source", "target") - .setResizeType(ResizeType.SPLIT) - .setSettings(Settings.builder() - .put("index.number_of_replicas", createWithReplicas ? 1 : 0) - .put("index.number_of_shards", 2).build()).get()); + .setResizeType(ResizeType.SPLIT) + .setSettings(Settings.builder() + .put("index.number_of_replicas", createWithReplicas ? 1 : 0) + .put("index.number_of_shards", 2) + .put("index.blocks.write", (String) null).build()).get()); ensureGreen(); final ClusterState state = client().admin().cluster().prepareState().get().getState(); @@ -431,8 +434,9 @@ public void testCreateSplitIndex() { if (createWithReplicas == false) { // bump replicas client().admin().indices().prepareUpdateSettings("target") - .setSettings(Settings.builder() - .put("index.number_of_replicas", 1)).get(); + .setSettings(Settings.builder() + .put("index.number_of_replicas", 1) + .put("index.blocks.write", (String) null)).get(); ensureGreen(); assertHitCount(client().prepareSearch("target").setSize(size).setQuery(new TermsQueryBuilder("foo", "bar")).get(), docs); } @@ -496,21 +500,23 @@ public void testCreateSplitWithIndexSort() throws Exception { // check that index sort cannot be set on the target index IllegalArgumentException exc = expectThrows(IllegalArgumentException.class, - () -> client().admin().indices().prepareResizeIndex("source", "target") - .setResizeType(ResizeType.SPLIT) - .setSettings(Settings.builder() - .put("index.number_of_replicas", 0) - .put("index.number_of_shards", 4) - .put("index.sort.field", "foo") - .build()).get()); + () -> client().admin().indices().prepareResizeIndex("source", "target") + .setResizeType(ResizeType.SPLIT) + .setSettings(Settings.builder() + .put("index.number_of_replicas", 0) + .put("index.number_of_shards", 4) + .put("index.sort.field", "foo") + .put("index.blocks.write", (String) null) + .build()).get()); assertThat(exc.getMessage(), containsString("can't override index sort when resizing an index")); // check that the index sort order of `source` is correctly applied to the `target` assertAcked(client().admin().indices().prepareResizeIndex("source", "target") - .setResizeType(ResizeType.SPLIT) - .setSettings(Settings.builder() - .put("index.number_of_replicas", 0) - .put("index.number_of_shards", 4).build()).get()); + .setResizeType(ResizeType.SPLIT) + .setSettings(Settings.builder() + .put("index.number_of_replicas", 0) + .put("index.number_of_shards", 4) + .put("index.blocks.write", (String) null).build()).get()); ensureGreen(); flushAndRefresh(); GetSettingsResponse settingsResponse = diff --git a/server/src/test/java/org/elasticsearch/action/admin/indices/shrink/ResizeRequestTests.java b/server/src/test/java/org/elasticsearch/action/admin/indices/shrink/ResizeRequestTests.java index 77ead591a01f2..adb2a6c4a4a99 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/indices/shrink/ResizeRequestTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/indices/shrink/ResizeRequestTests.java @@ -19,6 +19,7 @@ package org.elasticsearch.action.admin.indices.shrink; +import org.elasticsearch.action.ActionRequestValidationException; import org.elasticsearch.action.admin.indices.alias.Alias; import org.elasticsearch.action.admin.indices.create.CreateIndexRequest; import org.elasticsearch.action.admin.indices.create.CreateIndexRequestTests; @@ -29,14 +30,28 @@ import org.elasticsearch.index.RandomCreateIndexGenerator; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.test.hamcrest.ElasticsearchAssertions; +import org.hamcrest.collection.IsCollectionWithSize; import java.io.IOException; import static org.elasticsearch.cluster.metadata.IndexMetaData.SETTING_NUMBER_OF_SHARDS; import static org.elasticsearch.common.xcontent.ToXContent.EMPTY_PARAMS; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.hasSize; public class ResizeRequestTests extends ESTestCase { + public void testCopySettingsValidation() { + ResizeRequest request = new ResizeRequest(); + request.setSourceIndex("source"); + request.setTargetIndex(new CreateIndexRequest("target")); + request.setCopySettings(false); + final ActionRequestValidationException e = request.validate(); + assertNotNull(e); + assertThat(e.validationErrors(), hasSize(1)); + assertThat(e.validationErrors().get(0), equalTo("copySettings can not be set to [false]")); + } + public void testToXContent() throws IOException { { ResizeRequest request = new ResizeRequest("target", "source"); diff --git a/server/src/test/java/org/elasticsearch/rest/action/admin/indices/RestResizeHandlerTests.java b/server/src/test/java/org/elasticsearch/rest/action/admin/indices/RestResizeHandlerTests.java index 75071309458cc..13164e93e1a00 100644 --- a/server/src/test/java/org/elasticsearch/rest/action/admin/indices/RestResizeHandlerTests.java +++ b/server/src/test/java/org/elasticsearch/rest/action/admin/indices/RestResizeHandlerTests.java @@ -29,6 +29,8 @@ import java.io.IOException; import java.util.Collections; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.hasToString; import static org.mockito.Mockito.mock; public class RestResizeHandlerTests extends ESTestCase { @@ -36,27 +38,57 @@ public class RestResizeHandlerTests extends ESTestCase { public void testShrinkCopySettingsDeprecated() throws IOException { final RestResizeHandler.RestShrinkIndexAction handler = new RestResizeHandler.RestShrinkIndexAction(Settings.EMPTY, mock(RestController.class)); - final String copySettings = randomFrom("true", "false"); + final String copySettings = "true"; final FakeRestRequest request = new FakeRestRequest.Builder(NamedXContentRegistry.EMPTY) .withParams(Collections.singletonMap("copy_settings", copySettings)) .withPath("source/_shrink/target") .build(); handler.prepareRequest(request, mock(NodeClient.class)); - assertWarnings("parameter [copy_settings] is deprecated but was [" + copySettings + "]"); + assertWarnings("parameter [copy_settings] is deprecated but was [true]"); + } + + public void testShrinkCopySettingsFalse() { + final RestResizeHandler.RestShrinkIndexAction handler = + new RestResizeHandler.RestShrinkIndexAction(Settings.EMPTY, mock(RestController.class)); + final String copySettings = "false"; + final FakeRestRequest request = + new FakeRestRequest.Builder(NamedXContentRegistry.EMPTY) + .withParams(Collections.singletonMap("copy_settings", copySettings)) + .withPath("source/_split/target") + .build(); + final IllegalArgumentException e = + expectThrows(IllegalArgumentException.class, () -> handler.prepareRequest(request, mock(NodeClient.class))); + assertThat(e, hasToString(containsString("copy_settings can not be set to [false]"))); + assertWarnings("parameter [copy_settings] is deprecated but was [false]"); } public void testSplitCopySettingsDeprecated() throws IOException { final RestResizeHandler.RestSplitIndexAction handler = new RestResizeHandler.RestSplitIndexAction(Settings.EMPTY, mock(RestController.class)); - final String copySettings = randomFrom("true", "false"); + final String copySettings = "true"; final FakeRestRequest request = new FakeRestRequest.Builder(NamedXContentRegistry.EMPTY) .withParams(Collections.singletonMap("copy_settings", copySettings)) .withPath("source/_split/target") .build(); handler.prepareRequest(request, mock(NodeClient.class)); - assertWarnings("parameter [copy_settings] is deprecated but was [" + copySettings + "]"); + assertWarnings("parameter [copy_settings] is deprecated but was [true]"); + } + + public void testSplitCopySettingsFalse() { + final RestResizeHandler.RestSplitIndexAction handler = + new RestResizeHandler.RestSplitIndexAction(Settings.EMPTY, mock(RestController.class)); + final String copySettings = "false"; + final FakeRestRequest request = + new FakeRestRequest.Builder(NamedXContentRegistry.EMPTY) + .withParams(Collections.singletonMap("copy_settings", copySettings)) + .withPath("source/_split/target") + .build(); + final IllegalArgumentException e = + expectThrows(IllegalArgumentException.class, () -> handler.prepareRequest(request, mock(NodeClient.class))); + assertThat(e, hasToString(containsString("copy_settings can not be set to [false]"))); + assertWarnings("parameter [copy_settings] is deprecated but was [false]"); } }