Skip to content

Commit

Permalink
Deprecate not copy settings and explicitly disallow (#30404)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
jasontedor committed May 14, 2018
1 parent 9d1e687 commit 7910274
Show file tree
Hide file tree
Showing 14 changed files with 165 additions and 59 deletions.
2 changes: 2 additions & 0 deletions docs/CHANGELOG.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,8 @@ analysis module. ({pull}30397[#30397])
Highlighting::
* Limit analyzed text for highlighting (improvements) {pull}28808[#28808] (issues: {issue}16764[#16764], {issue}27934[#27934])
{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 in the RestClient. Prefer
these instead of the multi-argument versions. ({pull}29623[#29623])
Expand Down
19 changes: 15 additions & 4 deletions docs/reference/indices/shrink-index.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -62,11 +62,20 @@ the following request:

[source,js]
--------------------------------------------------
POST my_source_index/_shrink/my_target_index
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.

Expand Down Expand Up @@ -97,7 +106,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,
Expand Down Expand Up @@ -125,9 +134,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 8.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 7.x]

[float]
=== Monitoring the shrink process
Expand Down
10 changes: 6 additions & 4 deletions docs/reference/indices/split-index.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,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
Expand Down Expand Up @@ -152,7 +152,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>
Expand All @@ -175,9 +175,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 8.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 7.x]

[float]
=== Monitoring the split process
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
---
"Shrink index via API":
- skip:
version: " - 6.3.99"
reason: expects warnings that pre-6.4.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
Expand Down Expand Up @@ -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:
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
---
"Shrink index ignores target template mapping":
- skip:
version: " - 5.99.99"
reason: bug fixed in 5.6.0
version: " - 6.3.99"
reason: expects warnings that pre-6.4.0 will not send
features: "warnings"

- do:
cluster.state: {}
Expand Down Expand Up @@ -69,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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
"Copy settings during shrink index":
- skip:
version: " - 6.3.99"
reason: copy_settings did not exist prior to 6.4.0
reason: expects warnings that pre-6.4.0 will not send
features: "warnings"

- do:
Expand Down Expand Up @@ -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:
Expand All @@ -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:
Expand All @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,9 @@ setup:
---
"Split index via API":
- skip:
version: " - 6.0.99"
reason: Added in 6.1.0
version: " - 6.3.99"
reason: expects warnings that pre-6.4.0 will not send
features: "warnings"

# make it read-only
- do:
Expand All @@ -60,6 +61,8 @@ setup:
settings:
index.number_of_replicas: 0
index.number_of_shards: 2
warnings:
- "resize operations without copying settings is deprecated; set parameter [copy_settings] to [true] for future default behavior"

- do:
cluster.health:
Expand Down Expand Up @@ -103,8 +106,9 @@ setup:
---
"Create illegal split indices":
- skip:
version: " - 6.0.99"
reason: Added in 6.1.0
version: " - 6.3.99"
reason: expects warnings that pre-6.4.0 will not send
features: "warnings"

# try to do an illegal split with number_of_routing_shards set
- do:
Expand All @@ -119,6 +123,8 @@ setup:
index.number_of_replicas: 0
index.number_of_shards: 2
index.number_of_routing_shards: 4
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:
Expand All @@ -132,3 +138,5 @@ setup:
settings:
index.number_of_replicas: 0
index.number_of_shards: 3
warnings:
- "resize operations without copying settings is deprecated; set parameter [copy_settings] to [true] for future default behavior"
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
---
"Split index ignores target template mapping":
# - skip:
# version: " - 6.0.99"
# reason: Added in 6.1.0
# uncomment once AwaitsFix is resolved
- skip:
# when re-enabling uncomment the below skips
version: "all"
reason: "AwaitsFix'ing, see https://github.com/elastic/elasticsearch/issues/30503"
# version: " - 6.3.99"
# reason: expects warnings that pre-6.4.0 will not send
features: "warnings"

# create index
- do:
Expand Down Expand Up @@ -68,6 +68,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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
"Copy settings during split index":
- skip:
version: " - 6.3.99"
reason: copy_settings did not exist prior to 6.4.0
reason: expects warnings that pre-6.4.0 will not send
features: "warnings"

- do:
Expand Down Expand Up @@ -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:
Expand All @@ -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:
Expand All @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ public class ResizeRequest extends AcknowledgedRequest<ResizeRequest> implements
private CreateIndexRequest targetIndexRequest;
private String sourceIndex;
private ResizeType type = ResizeType.SHRINK;
private boolean copySettings = false;
private Boolean copySettings;

ResizeRequest() {}

Expand All @@ -80,6 +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);
}
assert copySettings == null || copySettings;
return validationException;
}

Expand All @@ -98,10 +99,10 @@ 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)) {
copySettings = in.readBoolean();
if (in.getVersion().before(Version.V_6_4_0)) {
copySettings = null;
} else {
copySettings = false;
copySettings = in.readOptionalBoolean();
}
}

Expand All @@ -113,8 +114,11 @@ 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);
// noinspection StatementWithEmptyBody
if (out.getVersion().before(Version.V_6_4_0)) {

} else {
out.writeOptionalBoolean(copySettings);
}
}

Expand Down Expand Up @@ -187,11 +191,14 @@ public ResizeType getResizeType() {
return type;
}

public void setCopySettings(final boolean copySettings) {
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;
}

public boolean getCopySettings() {
public Boolean getCopySettings() {
return copySettings;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,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
Expand Down
Loading

0 comments on commit 7910274

Please sign in to comment.