Skip to content

[ML] Persist/restore state for DFA classification#50040

Merged
dimitris-athanasiou merged 2 commits intoelastic:masterfrom
dimitris-athanasiou:restore-classification-state
Dec 12, 2019
Merged

[ML] Persist/restore state for DFA classification#50040
dimitris-athanasiou merged 2 commits intoelastic:masterfrom
dimitris-athanasiou:restore-classification-state

Conversation

@dimitris-athanasiou
Copy link
Copy Markdown
Contributor

@dimitris-athanasiou dimitris-athanasiou commented Dec 10, 2019 •
edited
Loading

This commit adds state persist/restore for data frame analytics classification jobs.

@elasticmachine
Copy link
Copy Markdown
Collaborator

elasticmachine commented Dec 10, 2019

Pinging @elastic/ml-core (:ml)

droberts195
droberts195 approved these changes Dec 11, 2019
View reviewed changes
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

przemekwitek
przemekwitek reviewed Dec 11, 2019
View reviewed changes
...in/core/src/main/java/org/elasticsearch/xpack/core/ml/dataframe/analyses/Classification.java Outdated
Copy link
Copy Markdown

@przemekwitek przemekwitek Dec 11, 2019

Choose a reason for hiding this comment

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

Shouldn't it be "_classification_state" instead?

Copy link
Copy Markdown
Contributor Author

@dimitris-athanasiou dimitris-athanasiou Dec 11, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

I wasn't sure whether the underlying state was similar so I ignored this. However, after talking to @tveasey about it we decided it'd be clearer to distinguish between them. So, I'll be raising a PR to address this in the c++ side. Thanks for raising it!

...-multi-node-tests/src/test/java/org/elasticsearch/xpack/ml/integration/ClassificationIT.java Outdated
Copy link
Copy Markdown

@przemekwitek przemekwitek Dec 11, 2019

Choose a reason for hiding this comment

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

Is it the same method as in RegressionIT? If so, it could be moved to MlNativeDataFrameAnalyticsIntegTestCase, just like assertInferenceModelPersisted.

@dimitris-athanasiou
[ML] Restore state for DFA classification
ff284c5 
This commit enables restoring state for data frame analytics
classification jobs.
@dimitris-athanasiou dimitris-athanasiou force-pushed the restore-classification-state branch from 2c21868 to c66bc6b Compare December 12, 2019 13:52
@dimitris-athanasiou dimitris-athanasiou changed the title [ML] Restore state for DFA classification [ML] Persist/restore state for DFA classification Dec 12, 2019
@dimitris-athanasiou
Copy link
Copy Markdown
Contributor Author

dimitris-athanasiou commented Dec 12, 2019

@droberts195, @przemekwitek I have adjusted the PR to handle different suffix per analysis for the state doc ids. Could you please take another look?

@dimitris-athanasiou
Copy link
Copy Markdown
Contributor Author

dimitris-athanasiou commented Dec 12, 2019

run elasticsearch-ci/2

@dimitris-athanasiou
Copy link
Copy Markdown
Contributor Author

dimitris-athanasiou commented Dec 12, 2019

run elasticsearch-ci/packaging-sample-matrix

droberts195
droberts195 approved these changes Dec 12, 2019
View reviewed changes
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

It would be good to delete orphaned data frame analytics state documents in the nightly maintenance in a followup PR.

@dimitris-athanasiou dimitris-athanasiou merged commit b805d95 into elastic:master Dec 12, 2019
@dimitris-athanasiou dimitris-athanasiou deleted the restore-classification-state branch December 12, 2019 15:27
dimitris-athanasiou added a commit to dimitris-athanasiou/elasticsearch that referenced this pull request Dec 12, 2019
@dimitris-athanasiou
[7.x] [ML] Persist/restore state for DFA classification (elastic#50040)
c4cd17c 
This commit adds state persist/restore for data frame analytics classification jobs.

Backport of elastic#50040
@dimitris-athanasiou dimitris-athanasiou mentioned this pull request Dec 12, 2019
Merged
dimitris-athanasiou added a commit that referenced this pull request Dec 13, 2019
@dimitris-athanasiou
[7.x] [ML] Persist/restore state for DFA classification (#50040) (#50147
e6cbcf7 
)

This commit adds state persist/restore for data frame analytics classification jobs.

Backport of #50040
przemekwitek
przemekwitek reviewed Dec 13, 2019
View reviewed changes
Copy link
Copy Markdown

@przemekwitek przemekwitek left a comment

Choose a reason for hiding this comment

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

LGTM

@dliappis dliappis mentioned this pull request Dec 13, 2019
Closed
SivagurunathanV pushed a commit to SivagurunathanV/elasticsearch that referenced this pull request Jan 23, 2020
@dimitris-athanasiou @SivagurunathanV
[ML] Persist/restore state for DFA classification (elastic#50040)
67d3cb7 
This commit adds state persist/restore for data frame analytics classification jobs.
This was referenced Feb 3, 2020
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@przemekwitek przemekwitek przemekwitek left review comments

@droberts195 droberts195 droberts195 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

>enhancement :ml Machine learning v7.6.0 v8.0.0-alpha1

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@dimitris-athanasiou @elasticmachine @droberts195 @przemekwitek @jakelandis