[ML] fix data frame analytics when there are no ML nodes but lazy node allocation is allowed #67840
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…e allocation is allowed
|
Pinging @elastic/ml-core (:ml) |
|
@elastic/ml-ui how will the data frame analytics wizard behave if it gets a |
|
@elasticmachine update branch |
| explainRequest, | ||
| explainListener); | ||
|
|
||
| Optional<DiscoveryNode> node = findMlNode(clusterService.state()); |
There was a problem hiding this comment.
Is there some common package the findMlNode method could go to?
There was a problem hiding this comment.
I am thinking of just changing the logic so that we always call _explain and _explain handles this.
| .getResponse() | ||
| .results() | ||
| .get(0) | ||
| .getState(), equalTo(DataFrameAnalyticsState.STARTING)); |
There was a problem hiding this comment.
Could there be a problem that this busy loop won't "catch" the moment in which the state is STARTING because analytics will advance to the next state too quickly?
There was a problem hiding this comment.
no. Because there is no ML node to assign to.
…elasticsearch into feature/ml-dfa-scaling-from-0
przemekwitek
approved these changes
Jan 25, 2021
| @@ -190,7 +225,7 @@ private void redirectToMlNode(PutDataFrameAnalyticsAction.Request request, | |||
| /** | |||
| * Finds the first available ML node in the cluster state. | |||
| */ | |||
| private static Optional<DiscoveryNode> findMlNode(ClusterState clusterState) { | |||
| static Optional<DiscoveryNode> findMlNode(ClusterState clusterState) { | |||
There was a problem hiding this comment.
Could this be made private again?
benwtrent
added a commit
to benwtrent/elasticsearch
that referenced
this pull request
Jan 25, 2021
…e allocation is allowed (elastic#67840) We cannot calculate memory size estimates if there are no ML nodes. But, if lazy nodes are enabled (or lazy starting in the analytics config), we should still be able to start the job. In _explain if there are no ML nodes, but there are lazy nodes (or the data frame analytics config allows lazy opening), we simply skip the memory estimate (returning the default of 1gb)
benwtrent
added a commit
to benwtrent/elasticsearch
that referenced
this pull request
Jan 25, 2021
…e allocation is allowed (elastic#67840) We cannot calculate memory size estimates if there are no ML nodes. But, if lazy nodes are enabled (or lazy starting in the analytics config), we should still be able to start the job. In _explain if there are no ML nodes, but there are lazy nodes (or the data frame analytics config allows lazy opening), we simply skip the memory estimate (returning the default of 1gb)
benwtrent
added a commit
that referenced
this pull request
Jan 25, 2021
…azy node allocation is allowed (#67840) (#67921) * [ML] fix data frame analytics when there are no ML nodes but lazy node allocation is allowed (#67840) We cannot calculate memory size estimates if there are no ML nodes. But, if lazy nodes are enabled (or lazy starting in the analytics config), we should still be able to start the job. In _explain if there are no ML nodes, but there are lazy nodes (or the data frame analytics config allows lazy opening), we simply skip the memory estimate (returning the default of 1gb)
benwtrent
added a commit
that referenced
this pull request
Jan 25, 2021
…zy node allocation is allowed (#67840) (#67920) * [ML] fix data frame analytics when there are no ML nodes but lazy node allocation is allowed (#67840) We cannot calculate memory size estimates if there are no ML nodes. But, if lazy nodes are enabled (or lazy starting in the analytics config), we should still be able to start the job. In _explain if there are no ML nodes, but there are lazy nodes (or the data frame analytics config allows lazy opening), we simply skip the memory estimate (returning the default of 1gb)
droberts195
added a commit
to droberts195/elasticsearch
that referenced
this pull request
Jan 28, 2021
The data frame analytics memory estimation process is very small and short-lived. Therefore, it isn't so bad for it to run on a node that's not an ML node. This PR allows the data frame analytics memory estimation process to run on data or ingest nodes in addition to ML nodes. If the node receiving the _explain request is any of these then it handles the request, avoiding a transfer within the cluster. Otherwise the request is transferred to an ML node if there is one. If there is no ML node then the request is transferred to a data or ingest node. (And it fails in the extremely unlikely event that the cluster has no ML, data or ingest nodes.) Replacement for elastic#67840
droberts195
added a commit
that referenced
this pull request
Jan 29, 2021
…8146) The data frame analytics memory estimation process is very small and short-lived. Therefore, it isn't so bad for it to run on a node that's not an ML node. This PR allows the data frame analytics memory estimation process to run on data or ingest nodes in addition to ML nodes. If the node receiving the _explain request is any of these then it handles the request, avoiding a transfer within the cluster. Otherwise the request is transferred to an ML node if there is one. If there is no ML node then the request is transferred to a data or ingest node. (And it fails in the extremely unlikely event that the cluster has no ML, data or ingest nodes.) Replacement for #67840
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We cannot calculate memory size estimates if there are no ML nodes.
But, if lazy nodes are enabled (or lazy starting in the analytics config), we should still be able to start the job.
This commit adds two predicates:
_explainif there are no ML nodes, but there are lazy nodes (or the data frame analytics config allows lazy opening), we simply skip the memory estimate (returning null)_startwe skip checking the memory estimate via_explainif there are no nodes but lazy starting is allowed via the lazy node count or the data frame analytics config.