Add delayed datacheck to the datafeed job runner #35387

benwtrent · 2018-11-08T16:51:49Z

This adds a latent data check to the DatafeedJobwhen it runs real-time jobs.

It can optionally be enabled via the DatafeedConfig.

Two new options added to the DatafeedConfig object, client side, and docs have been updated accordingly.

Couple of decision points I could not really settle on:

Should we set a default delayed_data_check_window if should_run_delayed_data_check is true?
Should we put sane limits on delayed_data_check_window outside of the current bucket length checks? If so, what are those sane limits?
Should we attempt to put the time of the missing data in the Audit? Maybe put the timestamp of the latest bucket missing data?
How often should we check for this missing data?

The integration tests are done within DelayedDataDetectorIT previously included, that covered the actual looking up of missing data and doing real queries, etc. against a cluster.

Consequently, I just mocked out DelayedDataDetector in the DatafeedJobTests class and thought that coverage adequate.

Screenshot of how it looks:

Relates to #35131

elasticmachine · 2018-11-08T16:51:51Z

Pinging @elastic/ml-core

davidkyle

Code LGTM I have some general comments about the behaviour

.../rest-high-level/src/test/java/org/elasticsearch/client/ml/datafeed/DatafeedConfigTests.java

x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/DatafeedJob.java

davidkyle · 2018-11-08T18:01:48Z

Should we set a default delayed_data_check_window if should_run_delayed_data_check is true?

I say no. If we are using the model where the user has to set both params then it's a problem is both aren't set, otherwise the behaviour may be unexpected. I do think should_run_delayed_data_check should be true by default with a sensible value of delayed_data_check_window

Should we put sane limits on delayed_data_check_window outside of the current bucket length checks? If so, what are those sane limits?

With a 10 minute bucket the current value of 10,000 buckets is over 64 days. Did you consider making the limit an absolute time value rather than a function of the bucket length.

Should we attempt to put the time of the missing data in the Audit? Maybe put the timestamp of the latest bucket missing data?

Yes please.
Is there anything that stops the same message being audited again and again each time the check is made?

How often should we check for this missing data?

Less frequently 🤷‍♂️

x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/DatafeedJob.java

benwtrent · 2018-11-08T18:40:18Z

@davidkyle when it comes to a sane upper limit I can see benefits to either using a true TimeValue or a multiple of the bucket span.

I do realize that it is way too large right now, the current limit is dictated by the number of documents that are searchable at a time and not by a realistic use case.

Yes please.
Is there anything that stops the same message being audited again and again each time the check is made?

There is nothing preventing this right now...but if we store the latest timestamp of the bucket with missing docs && how many docs are missing overall, we could simply check if the values have changed and skip writing the message so that the audit log does not get spammed.

I do think should_run_delayed_data_check should be true by default with a sensible value of delayed_data_check_window

Any suggestions on a sensible value?

benwtrent · 2018-11-09T14:40:29Z

@davidkyle Updated. Key updates

Interval check is now 15 minutes, or the data check window length, which ever is smaller. If the querydelay+frequency is longer than both, we will be doing a check every time.
Default values are 2 hours for window length, and true
Fixed some of the initialization of the DelayedDataChecker, I did not like how I was validating the window, so adjusted it so that it is never null, and is initialized in the constructor. This leads to clearer behavior.
Added the timestamp to the audit message and prevented audit spamming.

client/rest-high-level/src/main/java/org/elasticsearch/client/ml/datafeed/DatafeedConfig.java

davidkyle

LGTM

davidkyle · 2018-11-09T16:58:01Z

client/rest-high-level/src/main/java/org/elasticsearch/client/ml/datafeed/DatafeedConfig.java

+         * The window must be larger than the {@link org.elasticsearch.client.ml.job.config.AnalysisConfig#bucketSpan}, less than
+         * 24 hours, and span less than 10,000x buckets.
+         *
+         * @param delayedDataCheckWindow The time length in the past from the latest finalized bucket to look for latent data


nit missing full stop at the end of the sentence

benwtrent · 2018-11-13T17:42:59Z

Jenkins retest this please

dimitris-athanasiou

Looks good! Left a few recommendations.

client/rest-high-level/src/main/java/org/elasticsearch/client/ml/datafeed/DatafeedConfig.java

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedConfig.java

x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/DelayedDataDetector.java

x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/DatafeedJob.java

x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/DelayedDataDetector.java

benwtrent · 2018-11-14T17:57:57Z

Jenkins retest this please

...st-high-level/src/main/java/org/elasticsearch/client/ml/datafeed/DelayedDataCheckConfig.java

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedConfig.java

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedUpdate.java

...plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedJobValidator.java

...in/java/org/elasticsearch/xpack/ml/datafeed/delayeddatacheck/DelayedDataDetectorFactory.java

dimitris-athanasiou

LGTM I think this looks wonderful now, great work Ben!

benwtrent · 2018-11-15T14:33:53Z

Jenkins retest this please

* ML: Adding missing datacheck to datafeedjob * Adding client side and docs * Making adjustments to validations * Making values default to on, having more sensible limits * Intermittent commit, still need to figure out interval * Adjusting delayed data check interval * updating docs * Making parameter Boolean, so it is nullable * bumping bwc to 7 before backport * changing to version current * moving delayed data check config its own object * Separation of duties for delayed data detection * fixing checkstyles * fixing checkstyles * Adjusting default behavior so that null windows are allowed * Mentioning the default value * Fixing comments, syncing up validations

benwtrent added 3 commits November 7, 2018 16:41

ML: Adding missing datacheck to datafeedjob

2066230

Adding client side and docs

a74221c

Making adjustments to validations

bdf37b1

benwtrent added >feature v7.0.0 :ml Machine learning v6.6.0 labels Nov 8, 2018

davidkyle reviewed Nov 8, 2018

View reviewed changes

.../rest-high-level/src/test/java/org/elasticsearch/client/ml/datafeed/DatafeedConfigTests.java Show resolved Hide resolved

x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/DatafeedJob.java Outdated Show resolved Hide resolved

davidkyle reviewed Nov 8, 2018

View reviewed changes

x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/DatafeedJob.java Outdated Show resolved Hide resolved

benwtrent added 5 commits November 8, 2018 15:35

Making values default to on, having more sensible limits

a74421c

Merge branch 'master' into feature/ml-datafeed-job-check-missing-data

c075d26

Intermittent commit, still need to figure out interval

37c7319

Adjusting delayed data check interval

7f30afe

updating docs

b462a72

benwtrent commented Nov 9, 2018

View reviewed changes

client/rest-high-level/src/main/java/org/elasticsearch/client/ml/datafeed/DatafeedConfig.java Outdated Show resolved Hide resolved

Making parameter Boolean, so it is nullable

de21564

davidkyle approved these changes Nov 9, 2018

View reviewed changes

benwtrent added 6 commits November 9, 2018 13:00

Merge branch 'master' into feature/ml-datafeed-job-check-missing-data

f678265

Merge branch 'master' into feature/ml-datafeed-job-check-missing-data

faa5fe1

Merge branch 'master' into feature/ml-datafeed-job-check-missing-data

352a03f

bumping bwc to 7 before backport

4e00aba

changing to version current

f42f6a4

Merge branch 'master' into feature/ml-datafeed-job-check-missing-data

8c87189

dimitris-athanasiou reviewed Nov 13, 2018

View reviewed changes

moving delayed data check config its own object

061f9a3

benwtrent added 3 commits November 14, 2018 09:28

Separation of duties for delayed data detection

183f7e7

fixing checkstyles

9655add

fixing checkstyles

2becd81

dimitris-athanasiou reviewed Nov 14, 2018

View reviewed changes

benwtrent added 2 commits November 14, 2018 13:37

Adjusting default behavior so that null windows are allowed

47bbedb

Mentioning the default value

1fc77a9

dimitris-athanasiou reviewed Nov 15, 2018

View reviewed changes

...in/java/org/elasticsearch/xpack/ml/datafeed/delayeddatacheck/DelayedDataDetectorFactory.java Outdated Show resolved Hide resolved

dimitris-athanasiou approved these changes Nov 15, 2018

View reviewed changes

Fixing comments, syncing up validations

49de320

Merge branch 'master' into feature/ml-datafeed-job-check-missing-data

55d0980

benwtrent merged commit f7ada9b into elastic:master Nov 15, 2018

benwtrent deleted the feature/ml-datafeed-job-check-missing-data branch November 15, 2018 19:32

benwtrent mentioned this pull request Jan 4, 2019

[ML] Determine when data is missing from a bucket due to Ingest latency #35131

Closed

jimczi added v7.0.0-beta1 and removed v7.0.0 labels Feb 7, 2019

benwtrent mentioned this pull request May 10, 2019

[ML] adding delayed_data_check_config to datafeed update docs #42095

Merged

Add delayed datacheck to the datafeed job runner #35387

Add delayed datacheck to the datafeed job runner #35387

Uh oh!

Conversation

benwtrent commented Nov 8, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

elasticmachine commented Nov 8, 2018

Uh oh!

davidkyle left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

davidkyle commented Nov 8, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

benwtrent commented Nov 8, 2018

Uh oh!

benwtrent commented Nov 9, 2018

Uh oh!

Uh oh!

davidkyle left a comment

Choose a reason for hiding this comment

Uh oh!

davidkyle Nov 9, 2018

Choose a reason for hiding this comment

Uh oh!

benwtrent commented Nov 13, 2018

Uh oh!

dimitris-athanasiou left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

benwtrent commented Nov 14, 2018

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dimitris-athanasiou left a comment

Choose a reason for hiding this comment

Uh oh!

benwtrent commented Nov 15, 2018

Uh oh!

Uh oh!

benwtrent commented Nov 8, 2018 •

edited

Loading

davidkyle commented Nov 8, 2018 •

edited

Loading