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

Allow for additive mappings update without creating a new version index #147237

Closed
rudolf opened this issue Dec 8, 2022 · 3 comments · Fixed by #149326
Closed

Allow for additive mappings update without creating a new version index #147237

rudolf opened this issue Dec 8, 2022 · 3 comments · Fixed by #149326
Assignees
Labels
Epic:ScaleMigrations Scale upgrade migrations to millions of saved objects Feature:Migrations Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc

Comments

@rudolf
Copy link
Contributor

rudolf commented Dec 8, 2022

In #124946 we will only migrate saved objects to a new version specific index if the mappings changed, otherwise we keep the existing index and only transform outdated documents.

An additional optimization would be to only create a new version specific index if the mappings change is incompatible with the existing index. For instance, adding a new field or a ignore_above to a keyword field can be done in place. Whereas, removing a field or changing a text field type to a numeric one isn't.

The behaviour for when a mappings change is compatible or not isn't always obvious. I can't remember the specifics but I have been surprised in the past that some changes I thought should be compatible weren't. So except for when a field is removed, Kibana should not be trying to determine whether a change is compatible, it should just send the update request to Elasticsearch. If the update succeeds we can proceed without a new index, if not, we need a new version specific index.

@rudolf rudolf added Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc Feature:Migrations labels Dec 8, 2022
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-core (Team:Core)

@afharo
Copy link
Member

afharo commented Dec 12, 2022

Dropping an apparently compatible but not really compatible use case: #147341 adds a compatible change (it adds a field to the mappings). However, a reindex is required so sorting by title.keyword works as expected.

@rudolf
Copy link
Contributor Author

rudolf commented Dec 12, 2022

In #145604 when Kibana starts up and detects the mappings of the index doesn't match what it expects, it will update the mappings, but also run an updateByQuery to "pickup" the new mappings. I didn't previously think of edge cases like this sub-keyword-field, but it is also possible that the document _source contains data in each document that never had an associated field in the mappings. So in general, we can't assume that adding a new field doesn't require updating the lucene index.

Assuming that a new field added to the mappings necessarily requires reading and updating all documents in that index is quite expensive. We could optimise it somewhat by only targeting documents of the type of the saved object that had its mappings update instead of updating all documents. E.g. if a dashboard changes its mappings in a compatible way, we need an updateByQuery on all type: 'dashboard' documents but we can ignore cases.

@rudolf rudolf changed the title Allow for compatible mappings update without creating a new version index Allow for additive mappings update without creating a new version index Dec 20, 2022
gsoldevila added a commit that referenced this issue Mar 1, 2023
…ex (#149326)

Fixes [#147237](#147237)

Based on the same principle as
[#147371](#147371), the goal of
this PR is to **avoid reindexing if possible**.
This time, the idea is to check whether the new mappings are still
compatible with the ones stored in ES.
To to so, we attempt to update the mappings in place in the existing
index, introducing a new `CHECK_COMPATIBLE_MAPPINGS` step:
* If the update operation fails, we assume the mappings are NOT
compatible, and we continue with the normal reindexing flow.
* If the update operation succeeds, we assume the mappings ARE
compatible, and we skip reindexing, just like
[#147371](#147371) does.


![image](https://user-images.githubusercontent.com/25349407/216979882-9fe9f034-b521-4171-b85d-50be6a13e179.png)

---------

Co-authored-by: kibanamachine <[email protected]>
bmorelli25 pushed a commit to bmorelli25/kibana that referenced this issue Mar 10, 2023
…ex (elastic#149326)

Fixes [elastic#147237](elastic#147237)

Based on the same principle as
[elastic#147371](elastic#147371), the goal of
this PR is to **avoid reindexing if possible**.
This time, the idea is to check whether the new mappings are still
compatible with the ones stored in ES.
To to so, we attempt to update the mappings in place in the existing
index, introducing a new `CHECK_COMPATIBLE_MAPPINGS` step:
* If the update operation fails, we assume the mappings are NOT
compatible, and we continue with the normal reindexing flow.
* If the update operation succeeds, we assume the mappings ARE
compatible, and we skip reindexing, just like
[elastic#147371](elastic#147371) does.


![image](https://user-images.githubusercontent.com/25349407/216979882-9fe9f034-b521-4171-b85d-50be6a13e179.png)

---------

Co-authored-by: kibanamachine <[email protected]>
@rudolf rudolf added the Epic:ScaleMigrations Scale upgrade migrations to millions of saved objects label Sep 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Epic:ScaleMigrations Scale upgrade migrations to millions of saved objects Feature:Migrations Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants