From e87c9d8064f05c7c309a0c698d5dd5a06da23131 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Fri, 4 May 2018 18:53:01 -0400 Subject: [PATCH 1/6] Deprecate not copy settings and explicitly disallow We want copying settings to be the default behavior. This commit deprecates not copying settings, and disallows explicitly not copying settings. This gives users a transition path to the future default behavior. --- .../test/indices.shrink/10_basic.yml | 6 +++ .../test/indices.shrink/20_source_mapping.yml | 7 +++ .../test/indices.shrink/30_copy_settings.yml | 24 +++++++--- .../test/indices.split/10_basic.yml | 21 ++++++-- .../test/indices.split/20_source_mapping.yml | 7 ++- .../test/indices.split/30_copy_settings.yml | 23 ++++++--- .../admin/indices/shrink/ResizeRequest.java | 23 ++++++--- .../indices/shrink/TransportResizeAction.java | 2 +- .../admin/indices/RestResizeHandler.java | 17 ++++--- .../indices/shrink/ResizeRequestTests.java | 26 ++++++++++ .../admin/indices/RestResizeHandlerTests.java | 48 +++++++++++++------ 11 files changed, 155 insertions(+), 49 deletions(-) diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.shrink/10_basic.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.shrink/10_basic.yml index f53c88bcfca2e..4d98eade8f709 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.shrink/10_basic.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.shrink/10_basic.yml @@ -1,5 +1,9 @@ --- "Shrink index via API": + - skip: + version: " - 6.99.99" + reason: expects warnings that pre-7.0.0 will not send + features: "warnings" # creates an index with one document solely allocated on the master node # and shrinks it into a new index with a single shard # we don't do the relocation to a single node after the index is created @@ -62,6 +66,8 @@ body: settings: index.number_of_replicas: 0 + warnings: + - "resize operations without copying settings is deprecated; set parameter [copy_settings] to [true] for future default behavior" - do: cluster.health: diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.shrink/20_source_mapping.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.shrink/20_source_mapping.yml index d96e1dbdcb99b..07b3515b50c9e 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.shrink/20_source_mapping.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.shrink/20_source_mapping.yml @@ -1,5 +1,10 @@ --- "Shrink index ignores target template mapping": + - skip: + version: " - 6.99.99" + reason: expects warnings that pre-7.0.0 will not send + features: "warnings" + - do: cluster.state: {} # Get master node id @@ -65,6 +70,8 @@ body: settings: index.number_of_replicas: 0 + warnings: + - "resize operations without copying settings is deprecated; set parameter [copy_settings] to [true] for future default behavior" - do: cluster.health: 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..6e595921d7f6e 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 @@ -1,8 +1,8 @@ --- "Copy settings during shrink index": - skip: - version: " - 6.3.99" - reason: copy_settings did not exist prior to 6.4.0 + version: " - 6.99.99" + reason: expects warnings that pre-7.0.0 will not send features: "warnings" - do: @@ -47,8 +47,6 @@ settings: index.number_of_replicas: 0 index.merge.scheduler.max_thread_count: 2 - warnings: - - "parameter [copy_settings] is deprecated but was [true]" - do: cluster.health: @@ -64,20 +62,19 @@ - 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 do a actual shrink and do not copy settings (by default) - do: indices.shrink: index: "source" target: "no-copy-settings-target" wait_for_active_shards: 1 master_timeout: 10s - copy_settings: false body: settings: index.number_of_replicas: 0 index.merge.scheduler.max_thread_count: 2 warnings: - - "parameter [copy_settings] is deprecated but was [false]" + - "resize operations without copying settings is deprecated; set parameter [copy_settings] to [true] for future default behavior" - do: cluster.health: @@ -92,3 +89,16 @@ - 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 + + # now we do a actual shrink and try to set no copy settings + - do: + catch: /illegal_argument_exception/ + indices.shrink: + index: "source" + target: "explicit-no-copy-settings-target" + wait_for_active_shards: 1 + master_timeout: 10s + copy_settings: false + body: + settings: + index.number_of_replicas: 0 diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.split/10_basic.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.split/10_basic.yml index ee40cc36347b9..f64f51b64f021 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.split/10_basic.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.split/10_basic.yml @@ -33,8 +33,9 @@ setup: --- "Split index via API": - skip: - version: " - 6.0.99" - reason: Added in 6.1.0 + version: " - 6.99.99" + reason: expects warnings that pre-7.0.0 will not send + features: "warnings" # make it read-only - do: @@ -60,6 +61,8 @@ setup: settings: index.number_of_replicas: 0 index.number_of_shards: 4 + warnings: + - "resize operations without copying settings is deprecated; set parameter [copy_settings] to [true] for future default behavior" - do: cluster.health: @@ -105,7 +108,8 @@ setup: "Split from 1 to N": - skip: version: " - 6.99.99" - reason: Added in 7.0.0 + reason: expects warnings that pre-7.0.0 will not send + features: "warnings" - do: indices.create: index: source_one_shard @@ -159,6 +163,8 @@ setup: settings: index.number_of_replicas: 0 index.number_of_shards: 5 + warnings: + - "resize operations without copying settings is deprecated; set parameter [copy_settings] to [true] for future default behavior" - do: cluster.health: @@ -204,8 +210,9 @@ setup: --- "Create illegal split indices": - skip: - version: " - 6.0.99" - reason: Added in 6.1.0 + version: " - 6.99.99" + reason: expects warnings that pre-7.0.0 will not send + features: "warnings" # try to do an illegal split with number_of_routing_shards set - do: @@ -220,6 +227,8 @@ setup: index.number_of_replicas: 0 index.number_of_shards: 4 index.number_of_routing_shards: 8 + warnings: + - "resize operations without copying settings is deprecated; set parameter [copy_settings] to [true] for future default behavior" # try to do an illegal split with illegal number_of_shards - do: @@ -233,3 +242,5 @@ setup: settings: index.number_of_replicas: 0 index.number_of_shards: 6 + warnings: + - "resize operations without copying settings is deprecated; set parameter [copy_settings] to [true] for future default behavior" diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.split/20_source_mapping.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.split/20_source_mapping.yml index 38f36c405a1cb..dc7d83bf978f7 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.split/20_source_mapping.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.split/20_source_mapping.yml @@ -1,8 +1,9 @@ --- "Split index ignores target template mapping": - skip: - version: " - 6.0.99" - reason: Added in 6.1.0 + version: " - 6.99.99" + reason: expects warnings that pre-7.0.0 will not send + features: "warnings" # create index - do: @@ -64,6 +65,8 @@ settings: index.number_of_shards: 2 index.number_of_replicas: 0 + warnings: + - "resize operations without copying settings is deprecated; set parameter [copy_settings] to [true] for future default behavior" - do: cluster.health: 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..e0ace991f4f0d 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 @@ -1,8 +1,8 @@ --- "Copy settings during split index": - skip: - version: " - 6.3.99" - reason: copy_settings did not exist prior to 6.4.0 + version: " - 6.99.99" + reason: expects warnings that pre-7.0.0 will not send features: "warnings" - do: @@ -50,8 +50,6 @@ index.number_of_replicas: 0 index.number_of_shards: 2 index.merge.scheduler.max_thread_count: 2 - warnings: - - "parameter [copy_settings] is deprecated but was [true]" - do: cluster.health: @@ -67,21 +65,20 @@ - 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 do a actual shrink and do not copy settings (by default) - do: indices.split: index: "source" target: "no-copy-settings-target" wait_for_active_shards: 1 master_timeout: 10s - copy_settings: false body: settings: index.number_of_replicas: 0 index.number_of_shards: 2 index.merge.scheduler.max_thread_count: 2 warnings: - - "parameter [copy_settings] is deprecated but was [false]" + - "resize operations without copying settings is deprecated; set parameter [copy_settings] to [true] for future default behavior" - do: cluster.health: @@ -96,3 +93,15 @@ - 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 + + - do: + catch: /illegal_argument_exception/ + indices.split: + index: "source" + target: "explicit-no-copy-settings-target" + wait_for_active_shards: 1 + master_timeout: 10s + copy_settings: false + body: + settings: + index.number_of_replicas: 0 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..3db1b1a154684 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; 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 != null && copySettings == false) { + validationException = addValidationError("[copySettings] can not be explicitly set to [false]", validationException); + } return validationException; } @@ -98,10 +101,12 @@ public void readFrom(StreamInput in) throws IOException { } else { type = ResizeType.SHRINK; // BWC this used to be shrink only } - if (in.getVersion().onOrAfter(Version.V_6_4_0)) { + if (in.getVersion().before(Version.V_6_4_0)) { + copySettings = null; + } else if (in.getVersion().onOrAfter(Version.V_6_4_0) && in.getVersion().before(Version.V_7_0_0_alpha1)){ copySettings = in.readBoolean(); } else { - copySettings = false; + copySettings = in.readOptionalBoolean(); } } @@ -113,8 +118,12 @@ public void writeTo(StreamOutput out) throws IOException { if (out.getVersion().onOrAfter(ResizeAction.COMPATIBILITY_VERSION)) { out.writeEnum(type); } - if (out.getVersion().onOrAfter(Version.V_6_4_0)) { - out.writeBoolean(copySettings); + if (out.getVersion().before(Version.V_6_4_0)) { + + } else if (out.getVersion().onOrAfter(Version.V_6_4_0) && out.getVersion().before(Version.V_7_0_0_alpha1)) { + out.writeBoolean(copySettings == null ? false : copySettings); + } else { + out.writeOptionalBoolean(copySettings); } } @@ -187,11 +196,11 @@ public ResizeType getResizeType() { return type; } - public void setCopySettings(final boolean copySettings) { + public void setCopySettings(final Boolean copySettings) { this.copySettings = copySettings; } - public boolean getCopySettings() { + public Boolean getCopySettings() { return copySettings; } diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/shrink/TransportResizeAction.java b/server/src/main/java/org/elasticsearch/action/admin/indices/shrink/TransportResizeAction.java index 834ef15ce264d..040504ea97460 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/indices/shrink/TransportResizeAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/indices/shrink/TransportResizeAction.java @@ -190,7 +190,7 @@ static CreateIndexClusterStateUpdateRequest prepareCreateIndexRequest(final Resi .waitForActiveShards(targetIndex.waitForActiveShards()) .recoverFrom(metaData.getIndex()) .resizeType(resizeRequest.getResizeType()) - .copySettings(resizeRequest.getCopySettings()); + .copySettings(resizeRequest.getCopySettings() == null ? false : resizeRequest.getCopySettings()); } @Override 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..bc5db552b9dd2 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 @@ -48,17 +48,22 @@ public final RestChannelConsumer prepareRequest(final RestRequest request, final final ResizeRequest resizeRequest = new ResizeRequest(request.param("target"), request.param("index")); resizeRequest.setResizeType(getResizeType()); final String rawCopySettings = request.param("copy_settings"); - final boolean copySettings; + final Boolean copySettings; if (rawCopySettings == null) { copySettings = resizeRequest.getCopySettings(); + } else if (rawCopySettings.isEmpty()) { + copySettings = true; } else { - deprecationLogger.deprecated("parameter [copy_settings] is deprecated but was [" + rawCopySettings + "]"); - if (rawCopySettings.length() == 0) { - copySettings = true; - } else { - copySettings = Booleans.parseBoolean(rawCopySettings); + copySettings = Booleans.parseBoolean(rawCopySettings); + if (copySettings == false) { + throw new IllegalArgumentException("parameter [copy_settings] can not be explicitly set to [false]"); } } + if (copySettings == null) { + deprecationLogger.deprecated( + "resize operations without copying settings is deprecated; " + + "set parameter [copy_settings] to [true] for future default behavior"); + } resizeRequest.setCopySettings(copySettings); request.applyContentParser(resizeRequest::fromXContent); resizeRequest.timeout(request.paramAsTime("timeout", resizeRequest.timeout())); 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..70e7f083d43e9 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,39 @@ import org.elasticsearch.index.RandomCreateIndexGenerator; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.test.hamcrest.ElasticsearchAssertions; +import org.junit.Assert; import java.io.IOException; +import java.util.function.Consumer; 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() { + runTestCopySettingsValidation(false, e -> { + assertNotNull(e); + assertThat(e.validationErrors(), hasSize(1)); + assertThat(e.validationErrors().get(0), equalTo("[copySettings] can not be explicitly set to [false]")); + }); + + runTestCopySettingsValidation(null, Assert::assertNull); + runTestCopySettingsValidation(true, Assert::assertNull); + } + + private void runTestCopySettingsValidation(final Boolean copySettings, final Consumer consumer) { + final ResizeRequest request = new ResizeRequest(); + request.setSourceIndex("source"); + request.setTargetIndex(new CreateIndexRequest("target")); + request.setCopySettings(copySettings); + final ActionRequestValidationException e = request.validate(); + consumer.accept(e); + + } + 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..6e50b453a8104 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 @@ -20,15 +20,20 @@ package org.elasticsearch.rest.action.admin.indices; import org.elasticsearch.client.node.NodeClient; +import org.elasticsearch.common.Booleans; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.NamedXContentRegistry; import org.elasticsearch.rest.RestController; +import org.elasticsearch.rest.RestHandler; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.test.rest.FakeRestRequest; import java.io.IOException; import java.util.Collections; +import java.util.Locale; +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 +41,42 @@ 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 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 + "]"); + for (final String copySettings : new String[]{null, "", "true", "false"}) { + runTestResizeCopySettingsDeprecated(handler, "shrink", copySettings); + } } public void testSplitCopySettingsDeprecated() throws IOException { final RestResizeHandler.RestSplitIndexAction handler = new RestResizeHandler.RestSplitIndexAction(Settings.EMPTY, mock(RestController.class)); - final String copySettings = randomFrom("true", "false"); - final FakeRestRequest request = + for (final String copySettings : new String[]{null, "", "true", "false"}) { + runTestResizeCopySettingsDeprecated(handler, "split", copySettings); + } + } + + private void runTestResizeCopySettingsDeprecated( + final RestResizeHandler handler, final String resizeOperation, final String copySettings) throws IOException { + final FakeRestRequest.Builder builder = 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 + "]"); + .withPath(String.format(Locale.ROOT, "source/_%s/target", resizeOperation)); + if (copySettings != null) { + builder.withParams(Collections.singletonMap("copy_settings", copySettings)); + } + final FakeRestRequest request = builder.build(); + if ("false".equals(copySettings)) { + final IllegalArgumentException e = + expectThrows(IllegalArgumentException.class, () -> handler.prepareRequest(request, mock(NodeClient.class))); + assertThat(e, hasToString(containsString("parameter [copy_settings] can not be explicitly set to [false]"))); + } else { + handler.prepareRequest(request, mock(NodeClient.class)); + if (copySettings == null) { + assertWarnings( + "resize operations without copying settings is deprecated; " + + "set parameter [copy_settings] to [true] for future default behavior"); + } + } + } } From add8989d4bd2a9635841962feab0432bbe03fb86 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Fri, 4 May 2018 20:30:45 -0400 Subject: [PATCH 2/6] Adjust docs --- docs/CHANGELOG.asciidoc | 3 ++- docs/reference/indices/shrink-index.asciidoc | 10 ++++++---- docs/reference/indices/split-index.asciidoc | 10 ++++++---- 3 files changed, 14 insertions(+), 9 deletions(-) diff --git a/docs/CHANGELOG.asciidoc b/docs/CHANGELOG.asciidoc index aa7374c299333..1b5afe9d0b841 100644 --- a/docs/CHANGELOG.asciidoc +++ b/docs/CHANGELOG.asciidoc @@ -153,7 +153,8 @@ analysis module. ({pull}30397[#30397]) [float] === Enhancements -{ref-64}/breaking_64_api_changes.html#copy-source-settings-on-resize[Allow copying source settings on index resize operations] ({pull}30255[#30255]) +{ref-64}/breaking_64_api_changes.html#copy-source-settings-on-resize[Allow +copying source settings on index resize operations] ({pull}30255[#30255], {pull}30404[#30404]) Added new "Request" object flavored request methods. Prefer these instead of the multi-argument versions. ({pull}29623[#29623]) diff --git a/docs/reference/indices/shrink-index.asciidoc b/docs/reference/indices/shrink-index.asciidoc index 81d79c47472df..e5f90c3274ab2 100644 --- a/docs/reference/indices/shrink-index.asciidoc +++ b/docs/reference/indices/shrink-index.asciidoc @@ -62,7 +62,7 @@ the following request: [source,js] -------------------------------------------------- -POST my_source_index/_shrink/my_target_index +POST my_source_index/_shrink/my_target_index?copy_settings=true -------------------------------------------------- // CONSOLE // TEST[continued] @@ -97,7 +97,7 @@ and accepts `settings` and `aliases` parameters for the target index: [source,js] -------------------------------------------------- -POST my_source_index/_shrink/my_target_index +POST my_source_index/_shrink/my_target_index?copy_settings=true { "settings": { "index.number_of_replicas": 1, @@ -125,9 +125,11 @@ NOTE: By default, with the exception of `index.analysis`, `index.similarity`, and `index.sort` settings, index settings on the source index are not copied 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. +parameter `copy_settings=true` to the request. Note that `copy_settings` can not +be set to `false`. The parameter `copy_settings` will be removed in 9.0.0 -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, not copying settings is deprecated, copying settings will be +the default behavior in 8.x] [float] === Monitoring the shrink process diff --git a/docs/reference/indices/split-index.asciidoc b/docs/reference/indices/split-index.asciidoc index 58d34cfd9a705..8df7f342aa806 100644 --- a/docs/reference/indices/split-index.asciidoc +++ b/docs/reference/indices/split-index.asciidoc @@ -123,7 +123,7 @@ the following request: [source,js] -------------------------------------------------- -POST my_source_index/_split/my_target_index +POST my_source_index/_split/my_target_index?copy_settings=true { "settings": { "index.number_of_shards": 2 @@ -158,7 +158,7 @@ and accepts `settings` and `aliases` parameters for the target index: [source,js] -------------------------------------------------- -POST my_source_index/_split/my_target_index +POST my_source_index/_split/my_target_index?copy_settings=true { "settings": { "index.number_of_shards": 5 <1> @@ -181,9 +181,11 @@ NOTE: By default, with the exception of `index.analysis`, `index.similarity`, and `index.sort` settings, index settings on the source index are not copied 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. +parameter `copy_settings=true` to the request. Note that `copy_settings` can not +be set to `false`. The parameter `copy_settings` will be removed in 9.0.0 -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, not copying settings is deprecated, copying settings will be +the default behavior in 8.x] [float] === Monitoring the split process From 56ceb8901308d39ea48c1da5659938a8d89e57a7 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Sun, 6 May 2018 18:57:48 -0400 Subject: [PATCH 3/6] Fix docs test --- docs/reference/indices/shrink-index.asciidoc | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/docs/reference/indices/shrink-index.asciidoc b/docs/reference/indices/shrink-index.asciidoc index e5f90c3274ab2..5a03432012002 100644 --- a/docs/reference/indices/shrink-index.asciidoc +++ b/docs/reference/indices/shrink-index.asciidoc @@ -63,10 +63,19 @@ the following request: [source,js] -------------------------------------------------- POST my_source_index/_shrink/my_target_index?copy_settings=true +{ + "settings": { + "index.routing.allocation.require._name": null, <1> + "index.blocks.write": null <2> + } +} -------------------------------------------------- // CONSOLE // TEST[continued] +<1> Clear the allocation requirement copied from the source index. +<2> Clear the index write block copied from the source index. + The above request returns immediately once the target index has been added to the cluster state -- it doesn't wait for the shrink operation to start. From 6c0a153bc037df67dcca2b036dcb367e3f084081 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Mon, 7 May 2018 14:47:18 -0400 Subject: [PATCH 4/6] Newlines --- .../action/admin/indices/shrink/ResizeRequestTests.java | 1 - .../rest/action/admin/indices/RestResizeHandlerTests.java | 1 - 2 files changed, 2 deletions(-) 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 70e7f083d43e9..026af3c3e9f13 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 @@ -60,7 +60,6 @@ private void runTestCopySettingsValidation(final Boolean copySettings, final Con request.setCopySettings(copySettings); final ActionRequestValidationException e = request.validate(); consumer.accept(e); - } public void testToXContent() throws IOException { 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 6e50b453a8104..2c30184ee4e35 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 @@ -76,7 +76,6 @@ private void runTestResizeCopySettingsDeprecated( + "set parameter [copy_settings] to [true] for future default behavior"); } } - } } From 2c20228b46cff70fc2899c90b9dfd006af91f31c Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Wed, 9 May 2018 09:43:32 -0400 Subject: [PATCH 5/6] Move validation --- .../admin/indices/shrink/ResizeRequest.java | 7 +++-- .../indices/shrink/ResizeRequestTests.java | 31 +++++++++---------- 2 files changed, 18 insertions(+), 20 deletions(-) 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 3db1b1a154684..e510c0719df2d 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 @@ -80,9 +80,7 @@ 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 != null && copySettings == false) { - validationException = addValidationError("[copySettings] can not be explicitly set to [false]", validationException); - } + assert copySettings == null || copySettings; return validationException; } @@ -197,6 +195,9 @@ public ResizeType getResizeType() { } public void setCopySettings(final Boolean copySettings) { + if (copySettings != null && copySettings == false) { + throw new IllegalArgumentException("[copySettings] can not be explicitly set to [false]"); + } this.copySettings = copySettings; } 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 026af3c3e9f13..ba595de5215a3 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,7 +19,6 @@ 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; @@ -30,36 +29,34 @@ import org.elasticsearch.index.RandomCreateIndexGenerator; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.test.hamcrest.ElasticsearchAssertions; -import org.junit.Assert; import java.io.IOException; import java.util.function.Consumer; +import java.util.function.Supplier; 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; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.hasToString; public class ResizeRequestTests extends ESTestCase { public void testCopySettingsValidation() { - runTestCopySettingsValidation(false, e -> { - assertNotNull(e); - assertThat(e.validationErrors(), hasSize(1)); - assertThat(e.validationErrors().get(0), equalTo("[copySettings] can not be explicitly set to [false]")); + runTestCopySettingsValidation(false, r -> { + final IllegalArgumentException e = expectThrows(IllegalArgumentException.class, r::get); + assertThat(e, hasToString(containsString("[copySettings] can not be explicitly set to [false]"))); }); - runTestCopySettingsValidation(null, Assert::assertNull); - runTestCopySettingsValidation(true, Assert::assertNull); + runTestCopySettingsValidation(null, r -> assertNull(r.get().getCopySettings())); + runTestCopySettingsValidation(true, r -> assertTrue(r.get().getCopySettings())); } - private void runTestCopySettingsValidation(final Boolean copySettings, final Consumer consumer) { - final ResizeRequest request = new ResizeRequest(); - request.setSourceIndex("source"); - request.setTargetIndex(new CreateIndexRequest("target")); - request.setCopySettings(copySettings); - final ActionRequestValidationException e = request.validate(); - consumer.accept(e); + private void runTestCopySettingsValidation(final Boolean copySettings, final Consumer> consumer) { + consumer.accept(() -> { + final ResizeRequest request = new ResizeRequest(); + request.setCopySettings(copySettings); + return request; + }); } public void testToXContent() throws IOException { From 6aabd70c7767814ccfd1016cf839541b4a35840b Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Sat, 12 May 2018 05:22:50 -0400 Subject: [PATCH 6/6] Fix merge from master --- .../resources/rest-api-spec/test/indices.split/10_basic.yml | 2 +- .../rest-api-spec/test/indices.split/20_source_mapping.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.split/10_basic.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.split/10_basic.yml index 4cc92177750b6..635673c182f2f 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.split/10_basic.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.split/10_basic.yml @@ -112,7 +112,7 @@ setup: reason: "AwaitsFix'ing, see https://github.com/elastic/elasticsearch/issues/30503" # version: " - 6.99.99" # reason: expects warnings that pre-7.0.0 will not send - # features: "warnings" + features: "warnings" - do: indices.create: index: source_one_shard diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.split/20_source_mapping.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.split/20_source_mapping.yml index 46a889dd863da..433ac040dd1e4 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/indices.split/20_source_mapping.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/indices.split/20_source_mapping.yml @@ -6,7 +6,7 @@ reason: "AwaitsFix'ing, see https://github.com/elastic/elasticsearch/issues/30503" # version: " - 6.99.99" # reason: expects warnings that pre-7.0.0 will not send - # features: "warnings" + features: "warnings" # create index - do: