Skip to content
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

Shrink API should inherit (most of) original index's settings #28347

Closed
talevy opened this issue Jan 24, 2018 · 5 comments
Closed

Shrink API should inherit (most of) original index's settings #28347

talevy opened this issue Jan 24, 2018 · 5 comments
Assignees
Labels
:Data Management/Indices APIs APIs to create and manage indices and templates >enhancement

Comments

@talevy
Copy link
Contributor

talevy commented Jan 24, 2018

Currently, The shrunken target index inherits close to none of the original index's properties.

Since #25035 and #25380, other things are ignored.

At the moment, shrink relies on index templates, and just copies over mappings, and index.analysis.* and index.similarities.* from the original index.

It feels like it would make more sense for the shrunken index to be as close to a copy of the original
as possible. This would mean inheriting all the settings and mappings. Although, it should not inherit the aliases.

One way of solving this is to change the behavior and keep the new behavior behind a backwards-compatible query-param (maybe called copy_existing) that defaults to false.

this flag would be:

  • 6.x: log a deprecation warning when using shrink without copy_existing setting explicitly configured to true. Configuring it to false is not allowed.
  • 7.0: defaulted to true, still disallowed to be false, and deprecated
  • removed in 8.0
@talevy talevy added >enhancement discuss :Data Management/Indices APIs APIs to create and manage indices and templates labels Jan 24, 2018
@s1monw
Copy link
Contributor

s1monw commented Jan 24, 2018

++ sounds good to me

@colings86 colings86 self-assigned this Jan 25, 2018
@colings86 colings86 removed the discuss label Jan 25, 2018
@jasontedor
Copy link
Member

jasontedor commented Apr 13, 2018

It feels like it would make more sense for the shrunken index to be as close to a copy of the original as possible. This would mean inheriting all the settings and mappings. Although, it should not inherit the aliases.

To note, there probably needs to be a mechanism for settings to opt-out of being copied. For example, we would not want to copy cross-cluster replication settings as these would point to the pre-shrunken index.

@colings86
Copy link
Contributor

The process for removal is a little different than I envisaged (sorry for not mentioning this earlier, I had not thought about the removal process until @jasontedor opened #30321 to implement it.

I had thought that we would:

  • Add copySettings in 6.4.0 defaulted to false and add a deprecation warning if copySettings was not provided stating that the behaviour will change in 7.0. the actual copySettings parameter would not be deprecated though since we want people to use it in 6.4-6.last
  • In 7.0, change the default of copySettings to true, and make it so we emit deprecation warnings if the parameter is explicitly set on a resize request explaining that the parameter will be removed in 8.0. We wouldn't restrict which values of copySettings can be used in 7.0 though, just emit the warning if it is used.
  • In 8.0, remove the copySettings parameter completely

The worry I have with the approach listed above is that it puts the hard break in 7.0 since there is no way of not copying settings so to me it seems that its the same as just removing the parameter in 7.0?

@talevy
Copy link
Contributor Author

talevy commented May 3, 2018

I've updated the description to reflect latest comments above ^ as well as an offline discussion had with @jasontedor and @colings86.

Originally, the description stated that we will deprecate the setting in 6.x and had no mention of disallowing it to be set to false.

@talevy
Copy link
Contributor Author

talevy commented Jun 13, 2018

closed in the effort around introducing a notion of copy-settings #30255

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Data Management/Indices APIs APIs to create and manage indices and templates >enhancement
Projects
None yet
Development

No branches or pull requests

4 participants