ML: add migrate anomalies assistant #36643

benwtrent · 2018-12-14T14:21:00Z

This adds an endpoint, transport, and execution service for upgrading indices .ml-anomailes*.

We cannot use the upgrade plugin for these indices as they are already aliased for ML jobs.

The steps of the process are as follows

Create new write indices and include them in the read aliases
Point write aliases to the new write indices
make the old indices read only
reindex the old indices into new indices so that the old data is readable
adjust the read aliases so that they point to the newly reindexed indices
Delete the old indices

This PR does NOT contain docs for this new endpoint, or the HLRC addition. The PR is getting pretty bloated, I will open a new one for docs + HLRC after this one gets approved and merged.

elasticmachine · 2018-12-14T14:21:01Z

Pinging @elastic/ml-core

...gin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/ResultsIndexUpgradeAction.java

benwtrent · 2018-12-14T14:23:37Z

x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/ResultsIndexUpgradeService.java

+            );
+
+            // <1> Create the new write indices and set the read aliases to include them
+            createNewWriteIndicesIfNecessary(client, metaData, indexNameAndAliasProvider.newWriteIndices(),


The creation of the write indices, and adding them to the read alias are not done at the same time just in case the write indices already exist (from manual creation, or from a previous upgrade attempt), but the aliases still need set.

x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/ResultsIndexUpgradeService.java

x-pack/plugin/src/test/resources/rest-api-spec/api/ml.upgrade_job_results.json

…feed

droberts195

This looks great. I couldn't see any major problems on first pass but would like to have another look so please don't merge this year - we'll pick this up again in January.

I left a couple of minor comments about backporting.

One more thing I'd like to do before we merge this is go through all accesses to the results indices and make sure we're always using _search and never get, as get won't work when there are multiple indices. (This is important for #29946 as well as upgrade, which is why the upgrade work is such a big step towards being able to support rolling results indices.) But any changes for this aspect can be done in a separate PR.

droberts195 · 2018-12-18T17:18:51Z

x-pack/plugin/src/test/resources/rest-api-spec/api/ml.upgrade_job_results.json

+    "methods": [ "POST" ],
+    "url": {
+      "path": "/_ml/anomaly_detectors/results/_upgrade",
+      "paths": [ "/_ml/anomaly_detectors/results/_upgrade" ],


Remember to change to _xpack/ml in the backport to 6.x.

...lugin/ml/src/main/java/org/elasticsearch/xpack/ml/rest/results/RestUpgradeResultsAction.java

benwtrent · 2019-01-03T15:37:27Z

run the default distro tests

benwtrent · 2019-01-08T13:39:34Z

run the gradle build tests 1

hendrikmuhs · 2019-01-08T13:57:41Z

In the situation where one of the old indices was successfully reindexed (say index "A" -> "A2") , but another one failed (index "B" -> "B2"). We don't delete index "A" and we fail the migration and send the error to the user.

The user is free to retry the migration, we call the reindex for "A" -> "A2" again. With the parameters we send to the reindex call, we skip conflicts and ONLY create docs. We don't overwrite. So, nothing should happen with this attempt to reindex again.

Thanks @benwtrent, this answers my question. So if I re-run it "fills in" missing documents, so I re-read the whole source index but I do not write everything (only as needed). With other words, if I want - for some reason - to start from scratch I have to delete "A2", "B2", ... before running the migration again.

benwtrent · 2019-01-08T14:06:37Z

@hendrikmuhs yeah, delete whatever new indices you want to recreate. The caution is if the new write index is deleted, those docs are not stored anywhere else, so that newer data would be lost.

benwtrent · 2019-01-08T15:23:06Z

run the gradle build tests 1

benwtrent · 2019-01-08T15:58:08Z

run the gradle build tests 1

droberts195 · 2019-01-08T17:05:40Z

Could you point to where you saw this?

Ah, sorry, I missed that the read aliases are updated in two places.

New write indices are created, we add them to the read aliases:
https://github.com/elastic/elasticsearch/pull/36643/files#diff-7331c0bf65f9aef5ff4d2d3aca4aef95R247

In that case I think we have the reverse problem, which is that as the reindex progresses the UI will see more and more duplicate results, until eventually the reindex is complete, the read aliases are adjusted to only point at the new indices and the UI only sees one copy of each result. Whether this is a major problem in reality depends on how long it takes to get from step <1> to step <5>. If it's only a few seconds any user who sees duplicates will think it's a display quirk, press refresh and by that time the duplicates will have disappeared. But if it's many minutes then they may suffer for long enough to report it as a bug.

This potential problem shouldn't stop this PR being merged, as this PR creates a framework for the ML upgrade steps and we can iterate on the details of what it does in a subsequent PR.

davidkyle · 2019-01-08T16:26:06Z

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/MlUpgradeAction.java

+
+        @Override
+        public Task createTask(long id, String type, String action, TaskId parentTaskId, Map<String, String> headers) {
+            return new CancellableTask(id, type, action, getDescription(), parentTaskId, headers) {


I can't see where getDescription is overridden? The default implementation in TaskAwareRequest returns an empty string

davidkyle · 2019-01-08T17:07:14Z

x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/utils/TypedChainTaskExecutor.java

+     * Creates a new TypedChainTaskExecutor.
+     * Each chainedTask is executed in order serially and after each execution the continuationPredicate is tested.
+     *
+     * On failures teh failureShortCircuitPredicate is tested.


benwtrent · 2019-01-08T17:50:13Z

@droberts195 the user should never see duplicates with this process.
1 Write indexes are created
2 Read aliases include the new write indices
3 Write aliases are changed to ONLY point to the new write indices
4 Old read indices are set to read only
5 Old read indices are reindexed to new read indices
6 Read aliases are adjusted atomically to remove from the old read indices and add to the new read indices
7 old read indices are deleted

benwtrent · 2019-01-08T18:36:37Z

run the gradle build tests 2

benwtrent · 2019-01-08T21:52:59Z

Jenkins retest this please

x-pack/plugin/ml/qa/ml-with-security/build.gradle

...i-node-tests/src/test/java/org/elasticsearch/xpack/ml/integration/ResultsIndexUpgradeIT.java

droberts195

LGTM

the user should never see duplicates with this process.

Thanks for clarifying. I can see that now.

My next doubt is about whether renormalization might need changing in some way, but let's sort that out in a followup PR.

dimitris-athanasiou

LGTM

hendrikmuhs

LGTM

* ML: add migrate anomalies assistant * adjusting failure handling for reindex * Fixing request and tests * Adding tests to blacklist * adjusting test * test fix: posting data directly to the job instead of relying on datafeed * adjusting API usage * adding Todos and adjusting endpoint * Adding types to reindexRequest * removing unreliable "live" data test * adding index refresh to test * adding index refresh to test * adding index refresh to yaml test * fixing bad exists call * removing todo * Addressing remove comments * Adjusting rest endpoint name * making service have its own logger * adjusting validity check for newindex names * fixing typos * fixing renaming

benwtrent · 2019-01-25T17:43:46Z

Customer facing changes from this PR are reverted in: #37879

benwtrent added 2 commits December 14, 2018 07:52

ML: add migrate anomalies assistant

7effc73

adjusting failure handling for reindex

d578d1b

benwtrent added >feature v7.0.0 :ml Machine learning v6.6.0 labels Dec 14, 2018

benwtrent commented Dec 14, 2018

View reviewed changes

...gin/core/src/main/java/org/elasticsearch/xpack/core/ml/action/ResultsIndexUpgradeAction.java Outdated Show resolved Hide resolved

benwtrent commented Dec 14, 2018

View reviewed changes

x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/ResultsIndexUpgradeService.java Outdated Show resolved Hide resolved

benwtrent commented Dec 14, 2018

View reviewed changes

x-pack/plugin/src/test/resources/rest-api-spec/api/ml.upgrade_job_results.json Outdated Show resolved Hide resolved

benwtrent added 5 commits December 14, 2018 09:27

Fixing request and tests

a74bcfe

Adding tests to blacklist

9a43a54

adjusting test

b3fccc6

test fix: posting data directly to the job instead of relying on data…

07ed2a1

…feed

adjusting API usage

bf9e223

droberts195 added v6.7.0 and removed v6.6.0 labels Dec 18, 2018

droberts195 reviewed Dec 18, 2018

View reviewed changes

benwtrent added 6 commits January 2, 2019 07:48

Merge branch 'master' into feature/ml-migrate-anomalies-assistent

4a92d35

adding Todos and adjusting endpoint

064ac69

Adding types to reindexRequest

ba0df3a

removing unreliable "live" data test

a4c5e86

adding index refresh to test

28576fd

adding index refresh to test

8e9451e

benwtrent added 4 commits January 3, 2019 10:23

Merge branch 'master' into feature/ml-migrate-anomalies-assistent

cb9607d

adding index refresh to yaml test

1d6c66c

Merge branch 'master' into feature/ml-migrate-anomalies-assistent

fa56850

fixing bad exists call

b341424

Merge branch 'master' into feature/ml-migrate-anomalies-assistent

1594b49

davidkyle reviewed Jan 8, 2019

View reviewed changes

benwtrent added 2 commits January 8, 2019 14:27

Merge branch 'master' into feature/ml-migrate-anomalies-assistent

0650294

fixing typos

674463c

dimitris-athanasiou reviewed Jan 9, 2019

View reviewed changes

x-pack/plugin/ml/qa/ml-with-security/build.gradle Outdated Show resolved Hide resolved

...i-node-tests/src/test/java/org/elasticsearch/xpack/ml/integration/ResultsIndexUpgradeIT.java Outdated Show resolved Hide resolved

droberts195 approved these changes Jan 9, 2019

View reviewed changes

benwtrent added 2 commits January 9, 2019 07:40

Merge branch 'master' into feature/ml-migrate-anomalies-assistent

c4cf94f

fixing renaming

2d67895

dimitris-athanasiou approved these changes Jan 9, 2019

View reviewed changes

hendrikmuhs approved these changes Jan 9, 2019

View reviewed changes

benwtrent merged commit df3b58c into elastic:master Jan 9, 2019

benwtrent deleted the feature/ml-migrate-anomalies-assistent branch January 9, 2019 20:25

benwtrent added a commit that referenced this pull request Jan 10, 2019

ML: adjusting for backport of #36643

9db6f09

droberts195 mentioned this pull request Jan 14, 2019

Upgrade Assistant - Phase 2 - Reindexing elastic/kibana#26368

Closed

19 tasks

benwtrent mentioned this pull request Jan 25, 2019

ML: removing unnecessary upgrade code #37879

Merged

benwtrent added >non-issue and removed >feature labels Jan 25, 2019

colings86 added v7.0.0-beta1 and removed v7.0.0 labels Feb 7, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ML: add migrate anomalies assistant #36643

ML: add migrate anomalies assistant #36643

benwtrent commented Dec 14, 2018

elasticmachine commented Dec 14, 2018

benwtrent Dec 14, 2018

droberts195 left a comment

droberts195 Dec 18, 2018

benwtrent commented Jan 3, 2019

benwtrent commented Jan 8, 2019

hendrikmuhs commented Jan 8, 2019

benwtrent commented Jan 8, 2019

benwtrent commented Jan 8, 2019

benwtrent commented Jan 8, 2019

droberts195 commented Jan 8, 2019

davidkyle Jan 8, 2019

davidkyle Jan 8, 2019

benwtrent commented Jan 8, 2019 •

edited

Loading

benwtrent commented Jan 8, 2019

benwtrent commented Jan 8, 2019

droberts195 left a comment

dimitris-athanasiou left a comment

hendrikmuhs left a comment

benwtrent commented Jan 25, 2019

ML: add migrate anomalies assistant #36643

ML: add migrate anomalies assistant #36643

Conversation

benwtrent commented Dec 14, 2018

elasticmachine commented Dec 14, 2018

benwtrent Dec 14, 2018

Choose a reason for hiding this comment

droberts195 left a comment

Choose a reason for hiding this comment

droberts195 Dec 18, 2018

Choose a reason for hiding this comment

benwtrent commented Jan 3, 2019

benwtrent commented Jan 8, 2019

hendrikmuhs commented Jan 8, 2019

benwtrent commented Jan 8, 2019

benwtrent commented Jan 8, 2019

benwtrent commented Jan 8, 2019

droberts195 commented Jan 8, 2019

davidkyle Jan 8, 2019

Choose a reason for hiding this comment

davidkyle Jan 8, 2019

Choose a reason for hiding this comment

benwtrent commented Jan 8, 2019 • edited Loading

benwtrent commented Jan 8, 2019

benwtrent commented Jan 8, 2019

droberts195 left a comment

Choose a reason for hiding this comment

dimitris-athanasiou left a comment

Choose a reason for hiding this comment

hendrikmuhs left a comment

Choose a reason for hiding this comment

benwtrent commented Jan 25, 2019

benwtrent commented Jan 8, 2019 •

edited

Loading