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

[Backport to 1.3] restart bug fix #496

Merged
merged 3 commits into from
Apr 22, 2022

Conversation

kaituo
Copy link
Collaborator

@kaituo kaituo commented Apr 8, 2022

Description

backport #460 and #456 to 1.3 branch.

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.

kaituo and others added 2 commits April 8, 2022 12:58
* Fix restart HCAD detector bug

To prevent repeatedly cold starting a model due to sparse data, HCAD has a cache that remembers we have done cold start for a model. A second attempt to cold start will need to wait for 60 detector intervals. Previously, when stopping a detector, I forgot to clean the cache. So the cache remembers the model and won’t retry cold start after some time. This PR fixes the bug by cleaning the cache when stopping a detector.

Testing done:
1. added unit and integration tests.
2. manually reproduced the issue and verified the fix.

Signed-off-by: Kaituo Li <[email protected]>
@kaituo kaituo requested review from ylwu-amzn and amitgalitz April 8, 2022 20:42
@codecov-commenter
Copy link

codecov-commenter commented Apr 8, 2022

Codecov Report

Merging #496 (6854e94) into 1.3 (f51cd4e) will increase coverage by 0.06%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##                1.3     #496      +/-   ##
============================================
+ Coverage     77.68%   77.75%   +0.06%     
- Complexity     4111     4116       +5     
============================================
  Files           296      296              
  Lines         17669    17673       +4     
  Branches       1878     1878              
============================================
+ Hits          13727    13742      +15     
+ Misses         3043     3029      -14     
- Partials        899      902       +3     
Flag Coverage Δ
plugin 77.75% <100.00%> (+0.06%) ⬆️

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

Impacted Files Coverage Δ
...n/java/org/opensearch/ad/ml/EntityColdStarter.java 83.73% <100.00%> (+2.59%) ⬆️
...earch/ad/transport/DeleteModelTransportAction.java 96.15% <100.00%> (+0.32%) ⬆️
...ava/org/opensearch/ad/task/ADHCBatchTaskCache.java 88.88% <0.00%> (-1.24%) ⬇️
...opensearch/ad/indices/AnomalyDetectionIndices.java 72.28% <0.00%> (-0.19%) ⬇️
...java/org/opensearch/ad/task/ADBatchTaskRunner.java 82.06% <0.00%> (+0.30%) ⬆️
.../main/java/org/opensearch/ad/NodeStateManager.java 72.25% <0.00%> (+0.64%) ⬆️
...ain/java/org/opensearch/ad/model/ModelProfile.java 72.72% <0.00%> (+1.81%) ⬆️
...port/SearchAnomalyDetectorInfoTransportAction.java 62.22% <0.00%> (+6.66%) ⬆️

@ylwu-amzn ylwu-amzn changed the title Backport restart bug fix [Backport to 1.3] restart bug fix Apr 8, 2022
ylwu-amzn
ylwu-amzn previously approved these changes Apr 8, 2022
Copy link
Collaborator

@ylwu-amzn ylwu-amzn left a comment

Choose a reason for hiding this comment

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

LGTM.

@amitgalitz
Copy link
Member

Seems like test retry isn't helping here as much, is it just a lot more flaky on jdk8?

amitgalitz
amitgalitz previously approved these changes Apr 9, 2022
@kaituo
Copy link
Collaborator Author

kaituo commented Apr 11, 2022

Seems like test retry isn't helping here as much, is it just a lot more flaky on jdk8?

not sure. Will debug after oncall.

@kaituo kaituo dismissed stale reviews from amitgalitz and ylwu-amzn via fae13e5 April 21, 2022 22:19
@kaituo kaituo requested review from ylwu-amzn and amitgalitz April 22, 2022 00:23
@kaituo kaituo merged commit 0181605 into opensearch-project:1.3 Apr 22, 2022
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