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

Adds warning to the healthcheck when a migration is pending #30040

Closed
wants to merge 1 commit into from

Conversation

tylersmalley
Copy link
Contributor

@tylersmalley tylersmalley commented Feb 5, 2019

Since the index template was removed in #29268, we need to inform the user when they have done something to the index which requires a restart.

Here we wait until the migrations have been completed and check for pending required migrations with the health check.

To test you can delete the Kibana index after Kibana has started. You should begin to see warnings like the following:

server    log   [18:27:03.849] [warning] The Kibana server has pending migrations and should be restarted. This is most likely caused by removing or manually updating the underlying Kibana index.

This warning is currently limited to once every 30 seconds.

@tylersmalley tylersmalley added Team:Operations Team label for Operations Team v7.0.0 v6.7.0 labels Feb 5, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-operations

@tylersmalley
Copy link
Contributor Author

@chrisdavies, thoughts?

@elastic elastic deleted a comment from elasticmachine Feb 5, 2019
@elastic elastic deleted a comment from elasticmachine Feb 5, 2019
@elasticmachine
Copy link
Contributor

💚 Build Succeeded


if (server.kibanaMigrator && server.kibanaMigrator.hasMigrated === true) {
const { migrationVersion } = server.kibanaMigrator.documentMigrator;
const upTodate = await migrationsUpToDate(callWithInternalUser, kibanaIndex, migrationVersion, 1);
Copy link
Member

Choose a reason for hiding this comment

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

does it make sense to attach this to the kibanaMigrator class?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure what you mean. Do you mean move this logger logic into it or not use the kibanaMigrator here?

Copy link
Member

Choose a reason for hiding this comment

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

migrationsUpToDate on the surface looks like something kibanaMigrator would expose but they're resolved differently (import vs exposed on server object). i see it's used via a static class instead, feel free to ignore

@chrisdavies
Copy link
Contributor

I wonder how expensive this check is? I'm not sure if we want to be doing it every 30 seconds, as I think it requires a full index scan. But I don't know much about this kind of thing.

It's also going to be permanently spammy in the case that someone has applied an index template that affects Kibana's mappings. Maybe I should get this PR moving forward: #28252

Maybe we can make it a less-frequent check?

@jbudz
Copy link
Member

jbudz commented Feb 28, 2019

What if this was done on status change from red to green instead of as a health check?

@elasticmachine
Copy link
Contributor

💔 Build Failed

@snide snide removed v7.0.0 labels Apr 10, 2019
@jbudz jbudz removed the v6.7.0 label May 29, 2019
@tylersmalley
Copy link
Contributor Author

Closing this for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Team:Operations Team label for Operations Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants