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

[DOCS] Simplify ML upgrade step #40006

Merged
merged 6 commits into from
Mar 25, 2019
Merged

[DOCS] Simplify ML upgrade step #40006

merged 6 commits into from
Mar 25, 2019

Conversation

lcawl
Copy link
Contributor

@lcawl lcawl commented Mar 13, 2019

This PR contains changes suggested in #39944

@elasticmachine
Copy link
Collaborator

Pinging @elastic/ml-core

Copy link
Contributor

@droberts195 droberts195 left a comment

Choose a reason for hiding this comment

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

I left one comment. I guess it shows that the previous instructions were completely unclear about:

there must be no machine learning jobs running during the upgrade

When I wrote that I was thinking of "running" as meaning "actively doing stuff", and you can prevent that by setting upgrade mode. But I guess most people would read "running" as meaning the same as "opened".

Maybe this suggests a different restructuring of the whole thing...


If your {ml} indices were created before {prev-major-version}, you must stop
all {dfeeds} and close all {ml} jobs then
<<reindex-upgrade,reindex the indices>>.
Copy link
Contributor

Choose a reason for hiding this comment

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

An alternative here is to temporarily halt the tasks associated with your {ml} jobs and {dfeeds} and prevent new jobs from opening by using the <<ml-set-upgrade-mode,set upgrade mode API>> during the reindex.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, that step was added to the reindexing page in https://github.com/lcawl/elasticsearch/pull/1/files, so I've rebased and augmented the info there.

@lcawl
Copy link
Contributor Author

lcawl commented Mar 14, 2019

Thanks for the feedback, @droberts195.

When I wrote that I was thinking of "running" as meaning "actively doing stuff"... But I guess most people would read "running" as meaning the same as "opened".

Is it easy for users to know when a job is "actively doing stuff"? If not, then I think it would cause more headache than help to make that differentiation.

@droberts195
Copy link
Contributor

Is it easy for users to know when a job is "actively doing stuff"?

If ML upgrade mode is set then jobs will not be "actively doing stuff". Upgrade mode was added as a way for users to tell jobs not to do anything during an upgrade without having to close them. One problem with closing jobs is that after upgrade you have to reopen them, and that assumes you can remember which ones were open before you started. This is a hard problem for some environments, for example Cloud.

@lcawl
Copy link
Contributor Author

lcawl commented Mar 15, 2019

Would it be appropriate to change the lead-in sentence from:

"Stop any machine learning jobs that are running. (Optional)"

To something like this:

Temporarily stop the tasks associated with active {ml} jobs and {dfeeds}. (Optional)

ifdef::include-xpack[]
If you use {ml-features} and you're migrating indices from a 6.5 or earlier
cluster, the job and {dfeed} configuration information are not stored in an
index. You must recreate your {ml} jobs in the new cluster. If you are migrating from a 6.6 or later cluster, it is a good idea to temporarily halt the tasks associated with your {ml} jobs and {dfeeds} and prevent new jobs from opening
Copy link
Contributor

Choose a reason for hiding this comment

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

The jobs won't open on the new cluster because whether jobs are open or closed is determined by persistent tasks that won't be brought across by the reindex.

So I would change "prevent new jobs from opening" to something like "prevent inconsistencies between different ML indices that are reindexed at slightly different times".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @droberts195 does that clarification apply to the "reindex in place" section too?

Copy link
Contributor

Choose a reason for hiding this comment

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

For reindex in place the persistent tasks are in the same cluster, so I think that clarification only applies to reindex from remote.

Copy link
Contributor

@droberts195 droberts195 left a comment

Choose a reason for hiding this comment

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

LGTM

@lcawl lcawl merged commit 58e885c into elastic:master Mar 25, 2019
@lcawl lcawl deleted the stopping-ml branch March 25, 2019 17:23
lcawl added a commit that referenced this pull request Mar 25, 2019
lcawl added a commit that referenced this pull request Mar 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>docs General docs changes :ml Machine learning v7.0.0-rc2 v7.2.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants