[ML] job saved objects initialization #82639

jgowdyelastic · 2020-11-04T17:54:27Z

Adds a check during the plugin's start to see if any ml-job saved objects exist. If they don't, and ML jobs do exist, it will run an initialization process to create them.

Also moves the mlLog service to a better location.

Remake of #82450

This was checked for breaking API changes and was labeled appropriately

jgowdyelastic · 2020-11-04T17:56:58Z

x-pack/plugins/ml/server/saved_objects/initialization.ts

+    // const { body } = await client.asInternalUser.count({
+    //   index: '.ml-config',
+    // });
+    // return body.count > 0;


From the original PR:

@droberts195 @legrego
This function needs to check that there is at least one ML job in existence.
I believe the fastest way to do this would be to perform a count on .ml-config. However, the kibana system user does not have read access to that index.
Instead i'm having to call both AD and DFA get endpoints to perform the same check (below).
Do you think it would be safe to give the kibana system user the ability to make this check?

@legrego wrote:
I'm not terribly familiar with the contents of .ml-config. It appears to be a listing of jobs? Is there anything else in that index?

A more direct question might be: Is there any information in that index that's not accessible via the ML APIs granted to users with the manage_ml cluster privilege? I'd be ok granting read access to the .ml-config index if this is true, since kibana_system already has that cluster privilege.

On the other hand, if there is information in this index that the kibana_system user currently doesn't have access to via other APIs, then I'd need a better understanding of that data before answering one way or another.

@droberts195 what are your thoughts on this?

As this PR is holding up a lot of other work, i'm going to leave question open for a future PR.
I'll leave in this commented code in case the decision is to use count.

elasticmachine · 2020-11-04T18:00:11Z

Pinging @elastic/ml-ui (:ml)

peteharverson

LGTM

jgowdyelastic · 2020-11-05T13:51:59Z

@elasticmachine merge upstream

x-pack/plugins/ml/server/saved_objects/initialization.ts

alvarezmelissa87

LGTM ⚡

legrego · 2020-11-06T13:20:26Z

x-pack/plugins/ml/server/saved_objects/checks.ts

-    }
-    return results;
-  }
-
  async function _loadAllJobSavedObjects() {
    const { body } = await client.asInternalUser.search<SearchResponse<SavedObjectJob>>({


I overlooked this in the first PR. We need to be using the saved objects client to retrieve all jobs, instead of the raw ES client to query the index.

The way this is currently written, it won't support users who have changed their kibana index, and it may return results from indices that match this pattern that aren't actually the "kibana index".

Using the Saved Objects Client will also ensure that the Kibana index has been fully initialized, and is ready for plugins to access.

I used a search here because I need a list of all saved objects across all spaces, even the ones the user doesn't have access to. As far as i'm aware the saved object client can't do that?
checkStatus needs to know if a job has a saved object, otherwise we'll end up with duplicates when repairing.

@legrego i've updated these searches so it now uses the saved objects client

legrego · 2020-11-06T13:21:51Z

x-pack/plugins/ml/server/saved_objects/initialization.ts

+
+  async function _jobSavedObjectsExist(size: number = 1) {
+    const { body } = await client.asInternalUser.search({
+      index: '.kibana*',


Same comment as earlier: we should be using the Saved Objects client for this operation

legrego · 2020-11-06T13:31:06Z

x-pack/plugins/ml/server/plugin.ts

+    // check whether the job saved objects exist
+    // and create them if needed.
+    const { initializeJobs } = jobSavedObjectsInitializationFactory(coreStart);
+    initializeJobs().finally(() => {


To improve visibility into the status of this operation, we could hook into Core's status service (exposed on CoreSetup). This would allow the platform, ML consumers, and end users to understand why jobs may be missing in the event of a failure here.

Don't consider this a blocker for merging, but I'd recommend at least exploring this in a followup. If nothing else, it will help us triage failures with the inevitable SDH comes in

"inevitable" excuse me?!

Ha, that wasn't at all a knock on your work here, sorry if it came across that way. Coordinating something like this with a distributed system is bound to have some growing pains. We continue to have them on our team with respect to the authorization system we've built, despite our best efforts.

i know, sorry! i just thought that was funny.

legrego · 2020-11-06T13:32:23Z

x-pack/plugins/ml/server/saved_objects/initialization.ts

+      await initSavedObjects();
+      mlLog.info('Job saved objects initialized for * space');
+    } catch (error) {
+      mlLog.error('Error Initializing jobs');


It'd be good to log the error that we caught to help triage failures in production. Otherwise we may never know why initialization failed.

added in d2800ab

legrego · 2020-11-06T13:52:25Z

x-pack/plugins/ml/server/saved_objects/initialization.ts

+  }
+
+  async function _needsInitializing() {
+    if (await _jobSavedObjectsExist()) {


Will we be adding a "repair" option to the management UI in a followup? I worry that a partially initialized set of jobs would confuse end users, and restarting the Kibana server wouldn't help them since we won't attempt initialization if a single SO exists.

yes, repairing the saved objects from the management UI will be coming very soon, for 7.11

legrego · 2020-11-06T13:54:52Z

x-pack/plugins/ml/server/saved_objects/initialization.ts

+      );
+      const { initSavedObjects } = repairFactory(client, jobSavedObjectService);
+      await initSavedObjects();
+      mlLog.info('Job saved objects initialized for * space');


How easy is it to get the count of initialized jobs? That might be nice to record in the log message

added in a959ee7

jgowdyelastic · 2020-11-09T10:13:30Z

@elasticmachine merge upstream

darnautov · 2020-11-09T13:39:21Z

x-pack/plugins/ml/server/saved_objects/initialization.ts

+    if (await _jobsExist()) {
+      // some ml jobs exist, we need to create those saved objects
+      return true;
+    }
+
+    // no ml jobs actually exist,
+    // that's why there were no saved objects
+    return false;


nit:

Suggested change

if (await _jobsExist()) {

// some ml jobs exist, we need to create those saved objects

return true;

}

// no ml jobs actually exist,

// that's why there were no saved objects

return false;

return await _jobsExist();

darnautov · 2020-11-09T13:40:50Z

x-pack/plugins/ml/server/saved_objects/checks.ts

@@ -388,5 +191,5 @@ export function checksFactory(
    });
  }

-  return { checkStatus, repairJobs, initSavedObjects };
+  return { checkStatus };


shall we name it getCurrentStatus instead?

darnautov · 2020-11-09T13:42:20Z

x-pack/plugins/ml/server/saved_objects/repair.ts

+) {
+  const { checkStatus } = checksFactory(client, jobSavedObjectService);
+
+  async function repairJobs(simulate: boolean = false) {


what about having thesimulate check on top and return the default object instead of checking in teach for loop?

darnautov · 2020-11-09T13:43:53Z

x-pack/plugins/ml/server/saved_objects/repair.ts

+    const tasks: Array<() => Promise<void>> = [];
+
+    const status = await checkStatus();
+    for (const job of status.jobs['anomaly-detector']) {


nit: as there is no break statements or async execution, what about using .filter and .forEach

jgowdyelastic · 2020-11-09T17:00:35Z

@elasticmachine merge upstream

jgowdyelastic · 2020-11-10T12:53:20Z

@elasticmachine merge upstream

legrego

This looks great! One last comment. When spaces is disabled, the management screen returns a 400/bad request when querying for jobs. For the time being, you'll unfortunately need to strip out namespaces: ['*'] from the find request when spaces is disabled:

note your internal checks for initialization/repair can optionally keep namespaces: ['*'] in place if that makes your life easier, but this user-facing call will have to be smart enough to exclude it

legrego

LGTM with the understanding that we'll be adding comprehensive test coverage in a followup prior to release. Nice work!

kibanamachine · 2020-11-10T21:01:17Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 51bbb2b

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`ml`	1242	1243	+1

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`ml`	6.7MB	6.7MB	+270.0B

Distributable file count

id	before	after	diff
`default`	42766	42769	+3

History

💚 Build #87137 succeeded 9324bdd
💚 Build #87098 succeeded 5c3d751
💚 Build #87058 succeeded f7f1e6a
💚 Build #87050 succeeded 3701886
💚 Build #86837 succeeded 216a9d2

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

* [ML] job saved objects initialization * fixing job count logic * adding missing files * attempting to fix build crash * fixing kibana.json * changes based on review * removing accidentally added export * adding intialization promise * use finally so errors dont stop initialization * function rename * removing duplicate header * adding job initialization count to log message * adding error to log message * moving initialization file * moving intialization file back again to fix git stash issues * removing .kibana index search * creating internal saved object client * code clean up * removing commented code * adding check for spaces enabled * adding ids to saved objects Co-authored-by: Kibana Machine <[email protected]>

* [ML] job saved objects initialization * fixing job count logic * adding missing files * attempting to fix build crash * fixing kibana.json * changes based on review * removing accidentally added export * adding intialization promise * use finally so errors dont stop initialization * function rename * removing duplicate header * adding job initialization count to log message * adding error to log message * moving initialization file * moving intialization file back again to fix git stash issues * removing .kibana index search * creating internal saved object client * code clean up * removing commented code * adding check for spaces enabled * adding ids to saved objects Co-authored-by: Kibana Machine <[email protected]> Co-authored-by: Kibana Machine <[email protected]>

jgowdyelastic added 6 commits November 4, 2020 17:52

[ML] job saved objects initialization

01b7621

fixing job count logic

f934953

adding missing files

46d0081

attempting to fix build crash

a6b230c

fixing kibana.json

d80b4a7

changes based on review

eb8a4f6

jgowdyelastic requested review from a team as code owners November 4, 2020 17:54

jgowdyelastic requested review from peteharverson, darnautov, legrego and alvarezmelissa87 November 4, 2020 17:54

jgowdyelastic mentioned this pull request Nov 4, 2020

[ML] job saved objects initialization #82450

Closed

1 task

jgowdyelastic commented Nov 4, 2020

View reviewed changes

jgowdyelastic self-assigned this Nov 4, 2020

jgowdyelastic added :ml Feature:Anomaly Detection ML anomaly detection Feature:Data Frame Analytics ML data frame analytics features release_note:enhancement review v7.11.0 v8.0.0 labels Nov 4, 2020

peteharverson approved these changes Nov 4, 2020

View reviewed changes

removing accidentally added export

27efbe3

jgowdyelastic removed the request for review from a team November 5, 2020 12:34

Merge branch 'master' into job-saved-object-initialization-2

9388579

alvarezmelissa87 reviewed Nov 5, 2020

View reviewed changes

x-pack/plugins/ml/server/saved_objects/initialization.ts Outdated Show resolved Hide resolved

alvarezmelissa87 approved these changes Nov 5, 2020

View reviewed changes

jgowdyelastic added 2 commits November 6, 2020 09:14

function rename

8c6751f

removing duplicate header

b9d45ff

legrego reviewed Nov 6, 2020

View reviewed changes

legrego mentioned this pull request Nov 6, 2020

Allow the repository to search across all namespaces #82863

Merged

jgowdyelastic added 2 commits November 6, 2020 16:50

adding job initialization count to log message

a959ee7

adding error to log message

d2800ab

kibanamachine and others added 3 commits November 9, 2020 05:13

Merge branch 'master' into job-saved-object-initialization-2

c323370

moving initialization file

63769d4

moving intialization file back again to fix git stash issues

fb6a991

darnautov reviewed Nov 9, 2020

View reviewed changes

kibanamachine and others added 2 commits November 9, 2020 12:00

Merge branch 'master' into job-saved-object-initialization-2

216a9d2

removing .kibana index search

3701886

kibanamachine and others added 4 commits November 10, 2020 07:53

Merge branch 'master' into job-saved-object-initialization-2

f7f1e6a

creating internal saved object client

5c3d751

code clean up

b839622

removing commented code

9324bdd

legrego self-requested a review November 10, 2020 16:44

legrego reviewed Nov 10, 2020

View reviewed changes

jgowdyelastic added 2 commits November 10, 2020 18:31

adding check for spaces enabled

b81496e

adding ids to saved objects

51bbb2b

legrego approved these changes Nov 10, 2020

View reviewed changes

jgowdyelastic merged commit 096acb4 into elastic:master Nov 10, 2020

jgowdyelastic mentioned this pull request Nov 10, 2020

[7.x] [ML] job saved objects initialization (#82639) #83115

Merged

jgowdyelastic deleted the job-saved-object-initialization-2 branch November 10, 2020 21:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ML] job saved objects initialization #82639

[ML] job saved objects initialization #82639

jgowdyelastic commented Nov 4, 2020

jgowdyelastic Nov 4, 2020 •

edited

Loading

jgowdyelastic Nov 4, 2020

jgowdyelastic Nov 6, 2020

elasticmachine commented Nov 4, 2020

peteharverson left a comment

jgowdyelastic commented Nov 5, 2020

alvarezmelissa87 left a comment

legrego Nov 6, 2020

jgowdyelastic Nov 6, 2020

jgowdyelastic Nov 10, 2020

legrego Nov 6, 2020

legrego Nov 6, 2020

jgowdyelastic Nov 6, 2020 •

edited

Loading

legrego Nov 6, 2020

jgowdyelastic Nov 6, 2020 •

edited

Loading

legrego Nov 6, 2020

jgowdyelastic Nov 6, 2020

legrego Nov 6, 2020

jgowdyelastic Nov 6, 2020

legrego Nov 6, 2020

jgowdyelastic Nov 6, 2020

jgowdyelastic commented Nov 9, 2020

darnautov Nov 9, 2020

darnautov Nov 9, 2020

darnautov Nov 9, 2020

darnautov Nov 9, 2020

jgowdyelastic commented Nov 9, 2020

jgowdyelastic commented Nov 10, 2020

legrego left a comment •

edited

Loading

legrego left a comment

kibanamachine commented Nov 10, 2020

[ML] job saved objects initialization #82639

[ML] job saved objects initialization #82639

Conversation

jgowdyelastic commented Nov 4, 2020

jgowdyelastic Nov 4, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elasticmachine commented Nov 4, 2020

peteharverson left a comment

Choose a reason for hiding this comment

jgowdyelastic commented Nov 5, 2020

alvarezmelissa87 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jgowdyelastic Nov 6, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jgowdyelastic Nov 6, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jgowdyelastic commented Nov 9, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jgowdyelastic commented Nov 9, 2020

jgowdyelastic commented Nov 10, 2020

legrego left a comment • edited Loading

Choose a reason for hiding this comment

legrego left a comment

Choose a reason for hiding this comment

kibanamachine commented Nov 10, 2020

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

Distributable file count

History

jgowdyelastic Nov 4, 2020 •

edited

Loading

jgowdyelastic Nov 6, 2020 •

edited

Loading

jgowdyelastic Nov 6, 2020 •

edited

Loading

legrego left a comment •

edited

Loading