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

Add reindex feature to Upgrade Assistant #27457

Merged
merged 56 commits into from
Jan 29, 2019

Conversation

joshdover
Copy link
Contributor

@joshdover joshdover commented Dec 18, 2018

Summary

Related to #26368
Fixes #18469

This PR introduces the MVP reindexing feature to the 7.0 Upgrade Assistant.

Scope

This first iteration includes the following features:

  • Reindex indices created in 5.x
  • Persistence to persist reindex operation independent of the user's browser
  • Automatic handling of a few required index changes at reindex time (more on this below)

In a second PR to be merged in for 6.7 I will add:

  • Ability to cancel a running reindex operation
  • Cleanup of reindex saved objects once after completion.
  • Localized UI
  • Upgrading of internal indices (eg. Security)

Automatic index changes

There are two key situations that we are going to handle for the cluster admin automatically:

  • Removal of _all meta field from mappings
  • Conversion of deprecated boolean fields ("yes", "off", etc.). This is accomplished with a painless script added to the reindex command if any boolean fields are detected in the mapping.

Why are we doing this?

  • In 5.x, any indices that were created by indexing data without defining mappings defaulted to using the _all meta field. Because of this, we expect this to be a very common issue when reindexing 5.x indices.
  • If we chose not to handle converting boolean fields, cluster admins are going to handle this situation regardless. If we don't automatically handle this for them, we're not really saving them from any headaches.

Both of these changes are potentially breaking changes depending on the index is being used. Before starting a reindex on an index that has either of these problems, the UI alerts the user of the issue, how we handle it, and how it may affect any applications or scripts using this index (see screenshot below). We also require that the user go through a second confirmation before starting the reindex.

Screenshots

screen shot 2019-01-16 at 5 07 46 pm

screen shot 2019-01-16 at 5 06 56 pm

screen shot 2019-01-16 at 5 07 58 pm

Checklist

Use strikethroughs to remove checklist items you don't feel are applicable to this PR.

For maintainers

@elasticmachine
Copy link
Contributor

💔 Build Failed

@joshdover joshdover force-pushed the upgrade-checkup-reindex branch from 53cf03c to 262adc2 Compare December 21, 2018 17:24
@elasticmachine
Copy link
Contributor

💔 Build Failed

@joshdover joshdover force-pushed the upgrade-checkup-reindex branch from 262adc2 to 639529c Compare January 2, 2019 23:22
@elasticmachine
Copy link
Contributor

💔 Build Failed

@joshdover joshdover requested a review from a team as a code owner January 3, 2019 20:51
@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@joshdover
Copy link
Contributor Author

retest

@elasticmachine
Copy link
Contributor

💔 Build Failed

@joshdover joshdover force-pushed the upgrade-checkup-reindex branch from a55c278 to 3b5fa5b Compare January 4, 2019 20:03
@elasticmachine
Copy link
Contributor

💔 Build Failed

@joshdover joshdover force-pushed the upgrade-checkup-reindex branch from 3b5fa5b to 39cb4bb Compare January 4, 2019 22:17
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@elasticmachine
Copy link
Contributor

💔 Build Failed

@joshdover joshdover force-pushed the upgrade-checkup-reindex branch from 1ecdcb5 to d838ff5 Compare January 8, 2019 16:29
@elasticmachine
Copy link
Contributor

💔 Build Failed

@joshdover joshdover force-pushed the upgrade-checkup-reindex branch from d838ff5 to 85ee5dc Compare January 8, 2019 16:42
@elasticmachine
Copy link
Contributor

💔 Build Failed

@joshdover
Copy link
Contributor Author

retest

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@joshdover joshdover force-pushed the upgrade-checkup-reindex branch from df028d3 to 34cc97d Compare January 9, 2019 18:21
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@elasticmachine
Copy link
Contributor

💔 Build Failed

@joshdover joshdover force-pushed the upgrade-checkup-reindex branch from 7487b0e to 4ca7c52 Compare January 10, 2019 18:28
@elasticmachine
Copy link
Contributor

💔 Build Failed

@joshdover joshdover closed this Jan 10, 2019
@joshdover joshdover reopened this Jan 10, 2019
@joshdover
Copy link
Contributor Author

@jbudz i18n is going to be in a follow-up PR. Want to get this in and forward-ported to master so that we can complete work for APM in 7.0

@jbudz
Copy link
Member

jbudz commented Jan 25, 2019

Is it expected for a new template put every second? The polling is updating .kibana?

[2019-01-25T15:09:45,021][INFO ][o.e.c.m.MetaDataIndexTemplateService] [XoHVpDr] adding template [kibana_index_template:.kibana] for index patterns [.kibana]

@joshdover
Copy link
Contributor Author

Is it expected for a new template put every second? The polling is updating .kibana?

[2019-01-25T15:09:45,021][INFO ][o.e.c.m.MetaDataIndexTemplateService] [XoHVpDr] adding template [kibana_index_template:.kibana] for index patterns [.kibana]

When I looked into this before, I found that this is from the current behavior of the Saved Object Service that @chrisdavies wants to remove in 7.0.

@jbudz
Copy link
Member

jbudz commented Jan 25, 2019

Might be useful to select multiple indices at once, e.g. I have 30 filebeat-* indices with the same format and want to queue them all (flyout clicks can make it sorta slow). Not worried about this just noting at I go through things

@joelgriffith
Copy link
Contributor

Pardon my noobishness: if I wanted to test this out (locally, for reporting purposes), would the required steps be:

  • Startup ES/Kibana @ 5.x.
  • Throw some data in it.
  • Tear down 5.x, spin up 7.x with this branch.
  • Initiate re-index (I'm assuming it's in management).
  • ???
  • Profit

@joshdover
Copy link
Contributor Author

@joelgriffith Mostly right. You'll want to spin up 6.7 (this is on 6.x branch right now) and go to Management > 7.0 Upgrade Assistant > Indices to find the UI.

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

throw new Error(`Indices with more than one mapping type are not supported in 7.0.`);
}

return mappings[mappingTypes[0]];
Copy link
Contributor

@tylersmalley tylersmalley Jan 28, 2019

Choose a reason for hiding this comment

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

In 7.0 we will want to add a warning for types which are not _doc and enforce on re-indexing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should be easy enough, I can create a follow up PR for 7.0 after this gets merged.

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@tylersmalley
Copy link
Contributor

I have gone through the code and done some pretty extensive functional testing on this PR, including adding a feature to tag and re-index APM specific indices. There are a few things left outstanding which will be handled in follow-up PR's.

@joelgriffith
Copy link
Contributor

Sorry for the massive delay @joshdover, but I was able to test this with some confidence locally! I think you're good to proceed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@joshdover joshdover force-pushed the upgrade-checkup-reindex branch from 95c76a3 to f4342df Compare January 29, 2019 15:30
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@joshdover joshdover merged commit 468eabb into elastic:6.x Jan 29, 2019
joshdover added a commit to joshdover/kibana that referenced this pull request Jan 29, 2019
* Fixup saved objects

* WIP reindex state machine

* WIP UI

* Separate status from current step

* Add error messages

* Copy types to CallWithInternalUser

* Use backend worker

* Add progress bar for large indices

* Cleanup worker implementation

* Add tests for ReindexService

* Fix types

* Fix CI

* Add support for mapping coerced boolean fields

* Add basic functional integration test

* Cleanup reindexing backend, add more checks

* Cleanup frontend code and add tests

* Support removal of _default_ mapping + add tests

* Add reindex warnings to reindex service

* Add confirmation modal for reindex warnings

* Cleanup flyout appearance

* Generate new index name intelligently

* Design tweaks

* Show reindex button in both grouping modes

* Add data archive integration tests

* Change flyout design to two step process

* Reorganize flyout files

* Use custom steps component

* Allow writes to indices if anything fails

* Misc cleanup

* Design edits

* Only show warnings panel when reindex has not begun

* Fix types

* Handle moving existing aliases

* Move integration tests to separate config

* Fix ReindexButton bugs when changing pages

* Fix polling service tests

* Refactor ReindexService and ML handling (backend only)

* Add credential caching and paused state to backend

* Add paused state to frontend

* Fix ts errors

* Update copy

* Add check for node version before ML step

* Update data archive format

* Update snapshots for React upgrade

* Handle _default_ mappings correctly in warning checks

* Use hashed state of reindex operation for credential storage

* Address tyler's comments

* Add watcher stop/starting

* Use json-stable-stringify and sha256 for CredentialStore

* Add types for json-stable-stringify
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants