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.

Sign up for GitHub

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

Jump to bottom

[ML] Don't install empty ML metadata on startup #30751

Merged
merged 1 commit into from
May 21, 2018
Merged

[ML] Don't install empty ML metadata on startup #30751

merged 1 commit into from
May 21, 2018

Conversation

droberts195
Copy link
Contributor

This change is to support rolling upgrade from a pre-6.3 default
distribution (i.e. without X-Pack) to a 6.3+ default distribution
(i.e. with X-Pack).

The ML metadata is no longer eagerly added to the cluster state
as soon as the master node has X-Pack available. Instead, it
is added when the first ML job is created.

As a result all methods that get the ML metadata need to be able
to handle the situation where there is no ML metadata in the
current cluster state. They do this by behaving as though an
empty ML metadata was present. This logic is encapsulated by
always asking for the current ML metadata using a static method
on the MlMetadata class.

Relates #30731

@droberts195
[ML] Don't install empty ML metadata on startup
04eb564 
This change is to support rolling upgrade from a pre-6.3 default
distribution (i.e. without X-Pack) to a 6.3+ default distribution
(i.e. with X-Pack).

The ML metadata is no longer eagerly added to the cluster state
as soon as the master node has X-Pack available.  Instead, it
is added when the first ML job is created.

As a result all methods that get the ML metadata need to be able
to handle the situation where there is no ML metadata in the
current cluster state.  They do this by behaving as though an
empty ML metadata was present.  This logic is encapsulated by
always asking for the current ML metadata using a static method
on the MlMetadata class.

Relates elastic#30731
@elasticmachine
Copy link
Collaborator

Pinging @elastic/ml-core

droberts195
droberts195 commented May 21, 2018
View reviewed changes
x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MachineLearningFeatureSet.java
@@ -132,15 +131,7 @@ public boolean enabled() {
@Override
public void usage(ActionListener<XPackFeatureSet.Usage> listener) {
ClusterState state = clusterService.state();
MlMetadata mlMetadata = state.getMetaData().custom(MLMetadataField.TYPE);

// Handle case when usage is called but MlMetadata has not been installed yet
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was never really worth handling this case differently to ML metadata existing but being empty, as this state only existed for a fraction of a second on first cluster startup. Now this state will exist for longer - until the first job is created - but handling it like empty ML metadata is actually closer to the historical behaviour.

dimitris-athanasiou
dimitris-athanasiou approved these changes May 21, 2018
View reviewed changes
Copy link
Contributor

@dimitris-athanasiou dimitris-athanasiou left a comment

Choose a reason for hiding this comment

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

LGTM This is so much cleaner!

@droberts195 droberts195 merged commit eaf672f into elastic:master May 21, 2018
@droberts195 droberts195 deleted the do_not_install_ml_metadata_on_startup branch May 21, 2018 13:29
droberts195 added a commit that referenced this pull request May 21, 2018
@droberts195
[ML] Don't install empty ML metadata on startup (#30751)
4f1b6fa 
This change is to support rolling upgrade from a pre-6.3 default
distribution (i.e. without X-Pack) to a 6.3+ default distribution
(i.e. with X-Pack).

The ML metadata is no longer eagerly added to the cluster state
as soon as the master node has X-Pack available.  Instead, it
is added when the first ML job is created.

As a result all methods that get the ML metadata need to be able
to handle the situation where there is no ML metadata in the
current cluster state.  They do this by behaving as though an
empty ML metadata was present.  This logic is encapsulated by
always asking for the current ML metadata using a static method
on the MlMetadata class.

Relates #30731
droberts195 added a commit that referenced this pull request May 21, 2018
@droberts195
[ML] Don't install empty ML metadata on startup (#30751)
76386da 
This change is to support rolling upgrade from a pre-6.3 default
distribution (i.e. without X-Pack) to a 6.3+ default distribution
(i.e. with X-Pack).

The ML metadata is no longer eagerly added to the cluster state
as soon as the master node has X-Pack available.  Instead, it
is added when the first ML job is created.

As a result all methods that get the ML metadata need to be able
to handle the situation where there is no ML metadata in the
current cluster state.  They do this by behaving as though an
empty ML metadata was present.  This logic is encapsulated by
always asking for the current ML metadata using a static method
on the MlMetadata class.

Relates #30731
@jasontedor jasontedor mentioned this pull request May 21, 2018
Closed
9 tasks
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request May 21, 2018
@jasontedor
Merge remote-tracking branch 'elastic/master' into packaging-script-o…
bd65664 
…ne-liners

* elastic/master:
  [TEST] Enable DEBUG logging on testAutoQueueSizingWithMax
  [ML] Don't install empty ML metadata on startup (elastic#30751)
  Add assertion on removing copy_settings (elastic#30748)
  bump lucene version for 6_3_0
  [DOCS] Mark painless execute api as experimental (elastic#30710)
  disable annotation processor for docs (elastic#30610)
  Add more script contexts (elastic#30721)
  Fix default shards count in create index docs (elastic#30747)
dnhatn added a commit that referenced this pull request May 21, 2018
@dnhatn
Merge branch 'master' into ccr
3f6434c 
* master:
  Reduce CLI scripts to one-liners (#30759)
  SQL: Preserve scoring in bool queries (#30730)
  QA: Switch rolling upgrade to 3 nodes (#30728)
  [TEST] Enable DEBUG logging on testAutoQueueSizingWithMax
  [ML] Don't install empty ML metadata on startup (#30751)
  Add assertion on removing copy_settings (#30748)
  bump lucene version for 6_3_0
  [DOCS] Mark painless execute api as experimental (#30710)
  disable annotation processor for docs (#30610)
  Add more script contexts (#30721)
  Fix default shards count in create index docs (#30747)
  Mute testCorruptFileThenSnapshotAndRestore
dnhatn added a commit that referenced this pull request May 21, 2018
@dnhatn
Merge branch '6.x' into ccr-6.x
f45a2eb 
* 6.x:
  SQL: Preserve scoring in bool queries (#30730)
  QA: Switch rolling upgrade to 3 nodes
  [ML] Don't install empty ML metadata on startup (#30751)
  bump lucene version for 6_3_0
  [DOCS] Mark painless execute api as experimental (#30710)
  Add more script contexts (#30721)
@droberts195 droberts195 mentioned this pull request May 22, 2018
Merged
martijnvg added a commit to martijnvg/elasticsearch that referenced this pull request May 22, 2018
@martijnvg
Merge remote-tracking branch 'es/ccr' into ccr_create_and_follow
139977e 
* es/ccr: (50 commits)
  Reduce CLI scripts to one-liners (elastic#30759)
  SQL: Preserve scoring in bool queries (elastic#30730)
  QA: Switch rolling upgrade to 3 nodes (elastic#30728)
  [TEST] Enable DEBUG logging on testAutoQueueSizingWithMax
  [ML] Don't install empty ML metadata on startup (elastic#30751)
  Add assertion on removing copy_settings (elastic#30748)
  bump lucene version for 6_3_0
  [DOCS] Mark painless execute api as experimental (elastic#30710)
  disable annotation processor for docs (elastic#30610)
  Add more script contexts (elastic#30721)
  Fix default shards count in create index docs (elastic#30747)
  Mute testCorruptFileThenSnapshotAndRestore
  Scripting: Remove getDate methods from ScriptDocValues (elastic#30690)
  Upgrade to Lucene-7.4.0-snapshot-59f2b7aec2 (elastic#30726)
  [Docs] Fix single page :docs:check invocation (elastic#30725)
  Docs: Add uptasticsearch to list of clients (elastic#30738)
  [DOCS] Removes out-dated x-pack/docs/en/index.asciidoc
  [DOCS] Removes redundant index.asciidoc files (elastic#30707)
  [TEST] Reduce forecast overflow to disk test memory limit (elastic#30727)
  Plugins: Remove meta plugins (elastic#30670)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dimitris-athanasiou dimitris-athanasiou dimitris-athanasiou approved these changes

Assignees
No one assigned
Labels
:ml Machine learning >refactoring v6.3.0 v6.4.0 v7.0.0-beta1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@droberts195 @elasticmachine @dimitris-athanasiou @colings86