start historical detector #355

ylwu-amzn · 2021-01-05T18:13:14Z

Issue #, if available:

Description of changes:

If a detector has detection date range, we call it "historical detector".

Use the same start detector API to start both realtime and historical detector.

We don't change realtime detector execution logic.
For historical detector, we will start an AD task and return the task id to client, then dispatch the task to a node with least load. The AD task will be split into smaller pieces, one piece has 1000 detection intervals by default, user can tune the piece size dynamically. AD task runner will execute the pieces sequentially.

AD task states

Test

./gradlew build
./gradlew integTest -PnumNodes=3

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

codecov · 2021-01-05T18:14:48Z

Codecov Report

Merging #355 (b3b71e2) into master (dfca231) will increase coverage by 1.48%.
The diff coverage is 86.17%.

@@             Coverage Diff              @@
##             master     #355      +/-   ##
============================================
+ Coverage     77.22%   78.70%   +1.48%     
- Complexity     2277     2462     +185     
============================================
  Files           214      224      +10     
  Lines         10242    10995     +753     
  Branches        909      943      +34     
============================================
+ Hits           7909     8654     +745     
+ Misses         1914     1899      -15     
- Partials        419      442      +23

Flag	Coverage Δ	Complexity Δ
cli	`79.27% <ø> (ø)`	`0.00 <ø> (ø)`
plugin	`78.66% <86.17%> (+1.60%)`	`0.00 <175.00> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ	Complexity Δ
...distroforelasticsearch/ad/constant/CommonName.java	`66.66% <ø> (ø)`	`1.00 <0.00> (ø)`
...on/opendistroforelasticsearch/ad/model/ADTask.java	`100.00% <ø> (ø)`	`57.00 <0.00> (ø)`
...relasticsearch/ad/model/DetectorInternalState.java	`91.66% <ø> (ø)`	`13.00 <0.00> (ø)`
...est/handler/IndexAnomalyDetectorActionHandler.java	`52.67% <0.00%> (+0.92%)`	`31.00 <0.00> (ø)`
...ransport/DeleteAnomalyDetectorTransportAction.java	`21.68% <0.00%> (ø)`	`3.00 <0.00> (ø)`
...ch/ad/transport/handler/DetectionStateHandler.java	`87.50% <ø> (ø)`	`8.00 <0.00> (ø)`
.../handler/IndexAnomalyDetectorJobActionHandler.java	`40.84% <36.36%> (+30.25%)`	`11.00 <1.00> (+8.00)`
...sticsearch/ad/indices/AnomalyDetectionIndices.java	`50.32% <62.50%> (+0.97%)`	`44.00 <3.00> (+4.00)`
...ch/ad/common/exception/LimitExceededException.java	`80.00% <66.66%> (-20.00%)`	`3.00 <1.00> (ø)`
...on/opendistroforelasticsearch/ad/stats/ADStat.java	`86.66% <66.66%> (-5.00%)`	`7.00 <1.00> (+1.00)`	⬇️
... and 49 more

ylwu-amzn · 2021-01-05T20:16:11Z

...java/com/amazon/opendistroforelasticsearch/ad/common/exception/ADTaskCancelledException.java

@@ -0,0 +1,30 @@
+/*
+ * Copyright 2020 Amazon.com, Inc. or its affiliates. All Rights Reserved.


./gradlew spotlessApply will reset copyright year as 2020 as configured in file spotless.license.java.

To make this PR clean, will send out a separate PR to update this license file and apply to all files. Will replace 2020 with $YEAR, then spotless can fill as current year automatically.

src/main/java/com/amazon/opendistroforelasticsearch/ad/feature/FeatureManager.java

src/main/java/com/amazon/opendistroforelasticsearch/ad/indices/AnomalyDetectionIndices.java

src/main/java/com/amazon/opendistroforelasticsearch/ad/task/ADBatchTaskRunner.java

src/main/java/com/amazon/opendistroforelasticsearch/ad/feature/FeatureManager.java

src/main/java/com/amazon/opendistroforelasticsearch/ad/task/ADTaskManager.java

src/test/java/com/amazon/opendistroforelasticsearch/ad/plugin/MockReindexPlugin.java

src/main/java/com/amazon/opendistroforelasticsearch/ad/model/AnomalyDetector.java

src/main/java/com/amazon/opendistroforelasticsearch/ad/feature/FeatureManager.java

src/main/java/com/amazon/opendistroforelasticsearch/ad/task/ADBatchTaskRunner.java

kaituo

Only finished one class FeatureManager. Will continue reviewing.

src/main/java/com/amazon/opendistroforelasticsearch/ad/feature/FeatureManager.java

src/main/java/com/amazon/opendistroforelasticsearch/ad/task/ADBatchTaskRunner.java

src/main/java/com/amazon/opendistroforelasticsearch/ad/AnomalyDetectorPlugin.java

src/main/java/com/amazon/opendistroforelasticsearch/ad/util/ParseUtils.java

kaituo · 2021-01-07T01:44:50Z

src/main/java/com/amazon/opendistroforelasticsearch/ad/feature/FeatureManager.java

+        long endTime
+    ) {
+        long maxTimeDifference = detector.getDetectorIntervalInMilliseconds() / 2;
+        Map<Long, Entry<Long, Optional<double[]>>> featuresMap = getNearbyPointsForShingle(detector, shingle, endTime, maxTimeDifference)


Why do we need to call getNearbyPointsForShingle which helps real time detectors to deal with uneven arrival of requests? You run in batches and your timestamp within an interval is fixed.

This is to handle sparse data, this method is used for imputing missing points in the shingle with neighboring points here.

In getNearbyPointsForShingle, the imputing distance is half of the interval, which does not apply to your case.

Do we really need this logic for historical detector?

This is from product/user experience consideration, we will not differentiate the historical and realtime detection, will keep the model/algorithm consistent.

We don't need to impute missing data which caused by run time jitter for historical detector, just need to impute the data hole in source data.

Discussed with kaituo, will simplify the code for historical detector currently. For the single flow, we can create new function to handle both historical and realtime detection.

seems that we have lots of ideas for the new universal workflow. Should we create a doc/issue to track them?

Yeah, as universal flow is just started, I will doc these ideas on my notebook and share with internal team first. Don't thinks it will benefit community by sharing such unconnected ideas on Github now as we don't even mention what universal flow is. Will create an RFC issue later once we finish research, and put those ideas on that Github issue. So user can know the big picture of background, our solution, then they can understand why we come up with these ideas.

src/main/java/com/amazon/opendistroforelasticsearch/ad/feature/FeatureManager.java

kaituo · 2021-01-11T19:04:01Z

Thanks for the change for FeatureManager, Yaliang. I approve the change to that file.

ylwu-amzn requested review from kaituo, yizheliu-amazon and weicongs-amazon January 5, 2021 18:13

start historical detector

bb1967b

ylwu-amzn force-pushed the pr_start_task branch from 3a98ede to bb1967b Compare January 5, 2021 18:39

ylwu-amzn commented Jan 5, 2021

View reviewed changes

yizheliu-amazon reviewed Jan 5, 2021

View reviewed changes

ylwu-amzn force-pushed the pr_start_task branch from 96411fe to 1b6ed34 Compare January 6, 2021 05:02

address comments

0257a36

ylwu-amzn force-pushed the pr_start_task branch from 1b6ed34 to 0257a36 Compare January 6, 2021 06:40

yizheliu-amazon approved these changes Jan 6, 2021

View reviewed changes

src/main/java/com/amazon/opendistroforelasticsearch/ad/model/AnomalyDetector.java Show resolved Hide resolved

weicongs-amazon reviewed Jan 6, 2021

View reviewed changes

src/main/java/com/amazon/opendistroforelasticsearch/ad/task/ADBatchTaskRunner.java Show resolved Hide resolved

src/main/java/com/amazon/opendistroforelasticsearch/ad/task/ADBatchTaskRunner.java Show resolved Hide resolved

kaituo reviewed Jan 7, 2021

View reviewed changes

ylwu-amzn force-pushed the pr_start_task branch from 463fce2 to d92879e Compare January 7, 2021 05:09

return get feature failure;add todos

7c9d4b9

ylwu-amzn force-pushed the pr_start_task branch from d92879e to 7c9d4b9 Compare January 7, 2021 05:46

Merge branch 'master' into pr_start_task

0d1c8e2

weicongs-amazon approved these changes Jan 8, 2021

View reviewed changes

simplify shingle method for historical detector

b3b71e2

ylwu-amzn merged commit 19b8f9d into opendistro-for-elasticsearch:master Jan 11, 2021

ylwu-amzn mentioned this pull request Jan 12, 2021

stop historical detector; support return AD task in get detector API #359

Merged

ohltyler added the feature new feature label Feb 2, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

start historical detector #355

start historical detector #355

ylwu-amzn commented Jan 5, 2021

codecov bot commented Jan 5, 2021 •

edited

Loading

ylwu-amzn Jan 5, 2021 •

edited

Loading

kaituo left a comment

kaituo Jan 7, 2021

ylwu-amzn Jan 7, 2021 •

edited

Loading

kaituo Jan 8, 2021

weicongs-amazon Jan 8, 2021

ylwu-amzn Jan 8, 2021

ylwu-amzn Jan 8, 2021

weicongs-amazon Jan 9, 2021

ylwu-amzn Jan 9, 2021 •

edited

Loading

kaituo commented Jan 11, 2021

		@@ -0,0 +1,30 @@
		/*
		* Copyright 2020 Amazon.com, Inc. or its affiliates. All Rights Reserved.

start historical detector #355

start historical detector #355

Conversation

ylwu-amzn commented Jan 5, 2021

Description of changes:

AD task states

Test

codecov bot commented Jan 5, 2021 • edited Loading

Codecov Report

ylwu-amzn Jan 5, 2021 • edited Loading

Choose a reason for hiding this comment

kaituo left a comment

Choose a reason for hiding this comment

kaituo Jan 7, 2021

Choose a reason for hiding this comment

ylwu-amzn Jan 7, 2021 • edited Loading

Choose a reason for hiding this comment

kaituo Jan 8, 2021

Choose a reason for hiding this comment

weicongs-amazon Jan 8, 2021

Choose a reason for hiding this comment

ylwu-amzn Jan 8, 2021

Choose a reason for hiding this comment

ylwu-amzn Jan 8, 2021

Choose a reason for hiding this comment

weicongs-amazon Jan 9, 2021

Choose a reason for hiding this comment

ylwu-amzn Jan 9, 2021 • edited Loading

Choose a reason for hiding this comment

kaituo commented Jan 11, 2021

codecov bot commented Jan 5, 2021 •

edited

Loading

ylwu-amzn Jan 5, 2021 •

edited

Loading

ylwu-amzn Jan 7, 2021 •

edited

Loading

ylwu-amzn Jan 9, 2021 •

edited

Loading