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

[ML] Job in index: Datafeed node selector #34194

Closed

Conversation

davidkyle
Copy link
Member

@davidkyle davidkyle commented Oct 1, 2018

Breaks the DatafeedNodeSelectors dependency on datafeed and job configuration defined in the cluster state.

Similar to #33994 the required configuration is added to the persistent task's parameters. TransportStartDatafeedAction now collects the required config and adds it to StartDatafeedAction.DatafeedParams. The new fields added to the params cannot be streamed in BWC safe way but this isn't an issue as TransportStartDatafeedAction is a master node action and StartDatafeedPersistentTasksExecutor.getAssignment is also only called on the master node (by the PersistentTasksClusterService).

DatafeedManager.run is changed to accept the config as a parameter rather than reading from the clusterstate, this has the additional benefit that the validated configs are used rather than re-reading the config some point after validation. However, the BWC breaks down here as the datafeed may start on a node that isn't upgraded (mixed cluster) and the required config will not have been streamed in StartDatafeedAction.DatafeedParams.

Discuss

I can think of 2 solutions to this, either re-read the missing config in DatafeedManager or prevent the datafeed from starting on an old node/in a mixed cluster state. I prefer the later as I think it will simplify the migration process. That change can sensibly be done as part of the migration/upgrade work.

@davidkyle davidkyle added >feature :ml Machine learning labels Oct 1, 2018
@elasticmachine
Copy link
Collaborator

Pinging @elastic/ml-core

@droberts195
Copy link
Contributor

The diff for this PR is showing a huge number of changes, including many that were in #33994. Please can you merge the feature branch into your PR branch because I think this will remove the duplicated differences.

@davidkyle davidkyle changed the base branch from feature/jindex-6.x to 6.x October 2, 2018 09:42
@davidkyle davidkyle changed the base branch from 6.x to feature/jindex-6.x October 2, 2018 09:43
@davidkyle
Copy link
Member Author

Closing due to a rebase snafu. This pr is replaced by #34218

@davidkyle davidkyle closed this Oct 2, 2018
@davidkyle davidkyle deleted the datafeed-selector branch October 2, 2018 10:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>feature :ml Machine learning
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants