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

Deprecation check for indices with multiple types #36952

Merged
merged 6 commits into from
Jan 10, 2019

Conversation

gwbrown
Copy link
Contributor

@gwbrown gwbrown commented Dec 21, 2018

Multiple types can only show up in indices from the 5.x series, but they
can't be reindexed quite as easily as indices from 5.x that only have
one mapping type. Therefore, we want to present a separate message about
indices with multiple mapping types.

Relates to #36024 and #35190

/cc @joshdover

Multiple types can only show up in indices from the 5.x series, but they
can't be reindexed quite as easily as indices from 5.x that only have
one mapping type. Therefore, we want to present a separate message about
indices with multiple mapping types.
@gwbrown gwbrown added >enhancement :Search Foundations/Mapping Index mappings, including merging and defining field types :Core/Features/Features labels Dec 21, 2018
@gwbrown gwbrown requested a review from jtibshirani December 21, 2018 23:01
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-features

@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search

@jtibshirani
Copy link
Contributor

Thanks @gwbrown! This seems very helpful.

I think that if the index metadata contains more than one mapping type, there are actually two possibilities:

  • There is more than one mapping type for documents, for example tweets and users. The current deprecation check provides information about this case.
  • There is one mapping type for documents, but the user has also defined the _default_ mapping type. This default mapping type is deprecated in 6.x, and can no longer be used when creating a 7.x index.

It might make sense to call out these two cases separately for clarity, and also because the second one tends to be easier to resolve (the _default_ mappings can just be folded into the singleton mapping definition).

If there are only two mappings, and one of them is _default_, then the
index can be reindexed as normal
@gwbrown
Copy link
Contributor Author

gwbrown commented Dec 27, 2018

@jtibshirani Thanks! I've updated the check to cover that case - it looks from my testing like if there are two mappings, and one of them is _default_, the index can just be reindexed as normal, so I left that case as the standard "This needs to be reindexed" message, but if there are at least two "real" (non-_default_) mappings, then the new message calling out multiple types will be returned. Does that sound right?

@jtibshirani
Copy link
Contributor

jtibshirani commented Jan 1, 2019

The check for multiple document types looks good to me. I'm not totally up-to-date on how our migration assistant works , but if a _default_ mapping is present I don't think that reindex can proceed exactly as normal. In particular, the _default_ mapping can't be set on 7.0 indices, so users should make sure to fold all mapping items from _default_ into their single document mapping (or else these mapping items will be missing from the reindexed 7.0 indices).

@joshdover
Copy link
Contributor

@gwbrown Related question: How does this check work for indices with the _all field enabled?

Seems to me that these indices also cannot be reindex automatically since the usage of that field will need to be updated as well. If Elasticsearch supported creating a mapping field with the name _all we could probably handle this, but that does not seem to be allowed.

@gwbrown
Copy link
Contributor Author

gwbrown commented Jan 2, 2019

@joshdover That's also a good point - I'll take a look at mappings that aren't allowed in indices created in 6.0, but are allowed in 6.0 for indices created in 5.x and see if there's anything else we're missing that should block the reindex assistant.

@jtibshirani
Copy link
Contributor

Regarding my comment #36952 (comment), @gwbrown pointed out that the reindex helper will automatically copy mappings from the old index to the new one. Because the mapping items from _default_ get copied to the concrete mapping type when that type is created, the reindex helper could simply drop the _default_ type automatically, and copy over the main document mappings without any user action required. I defer to you all on whether it's okay to do this all automatically, and whether it'd be good to issue a note about it.

Copy link
Contributor

@jtibshirani jtibshirani left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 for the changes relating to multiple document mapping types.

@gwbrown
Copy link
Contributor Author

gwbrown commented Jan 9, 2019

Thanks @jtibshirani! We do intend to have a bit more discussion around the how the Kibana's reindex assistant will handle _default_ mappings to make sure we're doing the correct thing, but as the code for that is in Kibana, that conversation will happen elsewhere.

Regarding @joshdover's comment about _all, after some discussion, that is a smaller change that users are less likely to need special guidance on than the removal of mapping types. Therefore, we are not going to add a special case to the check in this PR for _all, but may provide an option to handle it in the reindex assistant in Kibana.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>enhancement :Search Foundations/Mapping Index mappings, including merging and defining field types v6.7.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants