Skip to content

ML: update .ml-state actions to support > 1 index#37307

Merged
benwtrent merged 7 commits intoelastic:masterfrom
benwtrent:feature/ml-state-support-multiple-indices
Jan 11, 2019
Merged

ML: update .ml-state actions to support > 1 index#37307
benwtrent merged 7 commits intoelastic:masterfrom
benwtrent:feature/ml-state-support-multiple-indices

Conversation

@benwtrent
Copy link
Copy Markdown
Member

For smoother upgrades in the future, and the ability to have more than one state index, certain actions needed to be adjusted so that having more than one state index would not break key features.

This addresses some (not all) of the points in #29938

Namely:

  • Make sure that all get requests for state documents are replaced with ID query _search requests that ignore duplicates
  • Make sure all deletes can handle multiple indices
  • Make sure index template has a wildcard so new state indices get the template

@elasticmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/ml-core


bulkRequestBuilder.add(client.prepareDelete(AnomalyDetectorsIndex.jobResultsAliasedName(modelSnapshot.getJobId()),
ElasticsearchMappings.DOC_TYPE, ModelSnapshot.documentId(modelSnapshot)));
idsToDelete.addAll(modelSnapshot.stateDocumentIds());
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I think the 6.x version of this method also adds legacy v5.4 doc IDs. It's probably best to raise a separate PR for the 6.x backport so we can review the 6.x code separately. There may be other places where 5.x legacy is handled differently in 6.x.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Sounds good to me, handling these whacky IDs is tricky. In current, the way we write the IDs prevents us from having duplicate IDs in different Indexes, not sure that is the case in v5.

@benwtrent
Copy link
Copy Markdown
Member Author

run the gradle build tests 2

Copy link
Copy Markdown

@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

@benwtrent benwtrent merged commit 19a7e0f into elastic:master Jan 11, 2019
@benwtrent benwtrent deleted the feature/ml-state-support-multiple-indices branch January 11, 2019 14:03
benwtrent added a commit that referenced this pull request Jan 14, 2019
* ML: update .ml-state actions to support > 1 index (#37307)

* ML: Updating .ml-state calls to be able to support > 1 index

* Matching bulk delete behavior with dbq

* Adjusting state name

* refreshing indices before search

* fixing line length

* adjusting index expansion options

* Adjusting for backport

* fixing minor test bug

* fixing test
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.

4 participants