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

reset realtime task state #201

Merged

Conversation

ylwu-amzn
Copy link
Collaborator

Signed-off-by: Yaliang Wu [email protected]

Description

Reset realtime task state when get latest detector task. If job is disabled, we should set realtime task state as stopped.

Check List

  • Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: Yaliang Wu <[email protected]>
@ylwu-amzn ylwu-amzn requested review from kaituo and ohltyler September 2, 2021 17:55
@codecov-commenter
Copy link

codecov-commenter commented Sep 2, 2021

Codecov Report

Merging #201 (649ec2c) into main (bedc5b6) will increase coverage by 0.23%.
The diff coverage is 48.40%.

Impacted file tree graph

@@             Coverage Diff              @@
##               main     #201      +/-   ##
============================================
+ Coverage     73.48%   73.71%   +0.23%     
- Complexity     3410     3444      +34     
============================================
  Files           276      276              
  Lines         15327    15383      +56     
  Branches       1560     1567       +7     
============================================
+ Hits          11263    11340      +77     
+ Misses         3341     3322      -19     
+ Partials        723      721       -2     
Flag Coverage Δ
plugin 73.71% <48.40%> (+0.23%) ⬆️

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

Impacted Files Coverage Δ
.../main/java/org/opensearch/ad/cluster/HashRing.java 81.30% <ø> (-0.09%) ⬇️
src/main/java/org/opensearch/ad/model/ADTask.java 90.40% <0.00%> (-0.23%) ⬇️
...search/ad/rest/RestDeleteAnomalyResultsAction.java 19.04% <ø> (+0.86%) ⬆️
...est/handler/IndexAnomalyDetectorActionHandler.java 43.85% <0.00%> (ø)
...pensearch/ad/settings/AnomalyDetectorSettings.java 100.00% <ø> (ø)
...java/org/opensearch/ad/task/ADBatchTaskRunner.java 64.81% <ø> (-1.31%) ⬇️
...ava/org/opensearch/ad/task/ADHCBatchTaskCache.java 71.91% <ø> (+14.60%) ⬆️
...d/transport/AnomalyDetectorJobTransportAction.java 89.74% <ø> (ø)
.../handler/IndexAnomalyDetectorJobActionHandler.java 51.65% <8.33%> (-3.35%) ⬇️
...ain/java/org/opensearch/ad/task/ADTaskManager.java 44.59% <33.33%> (-0.91%) ⬇️
... and 13 more

ohltyler
ohltyler previously approved these changes Sep 3, 2021
@ylwu-amzn ylwu-amzn force-pushed the reset_realtime_task_state branch from d84f4c8 to 96b914a Compare September 3, 2021 16:40
@@ -89,7 +89,7 @@
private Map<String, String> detectorTasks;

// Use this field to cache all HC tasks. Key is detector id
private Map<String, ADHCBatchTaskCache> hcTaskCaches;
private Map<String, ADHCBatchTaskCache> hcBatchTaskCaches;
Copy link
Collaborator

Choose a reason for hiding this comment

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

What are the differences between hcBatchTaskCaches and batchTaskCaches? Do they have overlap contents? Is it that hcBatchTaskCaches have detector level task cache, while batchTaskCaches has entity level task cache (if it is a HC detector) and detector level task cache (if it is single-stream detector)? if yes, then I am more confused. Why cannot we combine these two maps?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Your understanding is correct. hcBatchTaskCaches: cache HC detector level information on coordinating node, will add more comments. I think the comment for batchTaskCaches is enough, you can read and suggest more if the comment is still not clear.

They don't have overlap contents. Combine these two maps will mix the coordinating node and worker node cache together. I don't prefer that way now. If no big concern or cons , how about we keep current design now?

@ylwu-amzn ylwu-amzn merged commit 3d265a9 into opensearch-project:main Sep 3, 2021
ohltyler pushed a commit to ohltyler/anomaly-detection-2 that referenced this pull request Sep 7, 2021
* reset realtime task state

Signed-off-by: Yaliang Wu <[email protected]>

* try to delete AD tasks and job when detector not found

* address comments

* add more comments

* fix bugs; tune max running entities setting; add more UT

* rename variable

* add more comment
ohltyler pushed a commit to ohltyler/anomaly-detection-2 that referenced this pull request Sep 7, 2021
* reset realtime task state

Signed-off-by: Yaliang Wu <[email protected]>

* try to delete AD tasks and job when detector not found

* address comments

* add more comments

* fix bugs; tune max running entities setting; add more UT

* rename variable

* add more comment
ohltyler pushed a commit to ohltyler/anomaly-detection-2 that referenced this pull request Sep 14, 2021
* reset realtime task state

Signed-off-by: Yaliang Wu <[email protected]>

* try to delete AD tasks and job when detector not found

* address comments

* add more comments

* fix bugs; tune max running entities setting; add more UT

* rename variable

* add more comment
ohltyler pushed a commit that referenced this pull request Sep 23, 2021
* reset realtime task state

Signed-off-by: Yaliang Wu <[email protected]>

* try to delete AD tasks and job when detector not found

* address comments

* add more comments

* fix bugs; tune max running entities setting; add more UT

* rename variable

* add more comment
ohltyler pushed a commit that referenced this pull request Sep 23, 2021
* reset realtime task state

Signed-off-by: Yaliang Wu <[email protected]>

* try to delete AD tasks and job when detector not found

* address comments

* add more comments

* fix bugs; tune max running entities setting; add more UT

* rename variable

* add more comment
kaituo added a commit to kaituo/anomaly-detection-1 that referenced this pull request Sep 25, 2021
kaituo added a commit to kaituo/anomaly-detection-1 that referenced this pull request Sep 25, 2021
kaituo added a commit that referenced this pull request Sep 27, 2021
* [1.1] Bump OpenSearch core to 1.1 in CI (#212)

Signed-off-by: Tyler Ohlsen <[email protected]>

* add thresholded rcf (#215)

Signed-off-by: lai <[email protected]>

* Integrating with ThresholdedRandomCutForest

This PR contains:

PRs (all approved)  related to integrating ThresholdedRandomCutForest:
#221
#222
#223
#224
#226
#227
#228

rebased PR:
#201
#202
#216

* pass the correct shingleSize to ThresholdedRandomCutForest

Previously, I used shingleSize 1 for externally shingled ThresholdedRandomCutForest because of the double multiplication with shingle size in RCF.  Now RCF has fixed the issue. This commits adds new RCF libraries from aws/random-cut-forest-by-aws#278 and passes the correct shingleSize to ThresholdedRandomCutForest.

This commits adds new RCF libraries from aws/random-cut-forest-by-aws#278 and passes the correct shingleSize to ThresholdedRandomCutForest.

Co-authored-by: Tyler Ohlsen <[email protected]>
Co-authored-by: Lai <[email protected]>
hamersu9t added a commit to hamersu9t/anomaly-detection that referenced this pull request Aug 10, 2024
* [1.1] Bump OpenSearch core to 1.1 in CI (#212)

Signed-off-by: Tyler Ohlsen <[email protected]>

* add thresholded rcf (#215)

Signed-off-by: lai <[email protected]>

* Integrating with ThresholdedRandomCutForest

This PR contains:

PRs (all approved)  related to integrating ThresholdedRandomCutForest:
opensearch-project/anomaly-detection#221
opensearch-project/anomaly-detection#222
opensearch-project/anomaly-detection#223
opensearch-project/anomaly-detection#224
opensearch-project/anomaly-detection#226
opensearch-project/anomaly-detection#227
opensearch-project/anomaly-detection#228

rebased PR:
opensearch-project/anomaly-detection#201
opensearch-project/anomaly-detection#202
opensearch-project/anomaly-detection#216

* pass the correct shingleSize to ThresholdedRandomCutForest

Previously, I used shingleSize 1 for externally shingled ThresholdedRandomCutForest because of the double multiplication with shingle size in RCF.  Now RCF has fixed the issue. This commits adds new RCF libraries from aws/random-cut-forest-by-aws#278 and passes the correct shingleSize to ThresholdedRandomCutForest.

This commits adds new RCF libraries from aws/random-cut-forest-by-aws#278 and passes the correct shingleSize to ThresholdedRandomCutForest.

Co-authored-by: Tyler Ohlsen <[email protected]>
Co-authored-by: Lai <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants