This repository has been archived by the owner on Aug 2, 2022. It is now read-only.
Improve profile API's error fetching efficiency #117
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Previously, profile API scans all anomaly result indices to get a detector's most recent error, which can cause performance bottleneck with large anomaly result indices. This PR improves this aspect via various efforts. First, when a detector is running, we only need to scan the current index, not all of the rolled over ones since we are interested in the latest error. Second, when a detector is disabled, we only need to scan the latest anomaly result indices created before the detector's enable time. Third, setting track total hits false makes ES terminate search early. ES will not try to count the number of documents and will be able to end the query as soon as N document have been collected per segment. Testing done: 1. patched a cluster with 1,000 detectors and 2GB anomaly result indices. Without the PR, scanning anomaly result indices 1000 times would timeout after 30 seconds. After the PR, we would not see the timeout. 2. A detector's error message can be on a rotated index. Adds a test case to makes sure we get error info from .opendistro-anomaly-results index that has been rolled over.
| int date = Integer.parseInt(m.group(3)); | ||
| // month starts with 0 | ||
| calendar.clear(); | ||
| calendar.set(year, month - 1, date); |
There was a problem hiding this comment.
will Jan. cause any issue? I guess in case of Jan., month is 1, not sure if this can cause any issue
There was a problem hiding this comment.
I would suggest you use current year/month/date to initialize calendar, and do calendar.add(Calendar.MONTH, -1) instead.
There was a problem hiding this comment.
will Jan. cause any issue? I guess in case of Jan., month is 1, not sure if this can cause any issue
just checked java doc. month for Jan is 0 here.
ylwu-amzn
reviewed
May 8, 2020
| // time | ||
| if (timestamp <= disabledTimeMillis && maxTimestamp <= timestamp) { | ||
| maxTimestamp = timestamp; | ||
| // we can have two rotations on the same day and we don't know which one has our data, so we keep all |
There was a problem hiding this comment.
One edge case: suppose detector interval is 1 minute
1.Detector last run is at 2020-05-07, 11:59:50 PM, then AD result indices rolled over as .opendistro-anomaly-results-history-2020.05.07-001
2.Detector next run will be 2020-05-08, 00:00:50 AM. If user stop the detector at 2020-05-08 00:00:10 AM, detector will not have AD result on 2020-05-08.
So this code change will check latest AD result index on 2020-05-08, as 2020-05-08 <= 2020-05-08 00:00:10 AM(disabledTime). But we can't find any AD result for this detector on 2020-05-08. How about we check last two days' AD result indices to make sure we can always get AD result? Similar to set monitor interval as 2*detector_interval
yizheliu-amazon
approved these changes
May 8, 2020
ylwu-amzn
approved these changes
May 9, 2020
kaituo
added a commit
to kaituo/anomaly-detection
that referenced
this pull request
May 19, 2020
…icsearch#117) Previously, profile API scans all anomaly result indices to get a detector's most recent error, which can cause performance bottleneck with large anomaly result indices. This PR improves this aspect via various efforts. First, when a detector is running, we only need to scan the current index, not all of the rolled over ones since we are interested in the latest error. Second, when a detector is disabled, we only need to scan the latest anomaly result indices created before the detector's enable time. Third, setting track total hits false makes ES terminate search early. ES will not try to count the number of documents and will be able to end the query as soon as N document have been collected per segment. Testing done: 1. patched a cluster with 1,000 detectors and 2GB anomaly result indices. Without the PR, scanning anomaly result indices 1000 times would timeout after 30 seconds. After the PR, we would not see the timeout. 2. A detector's error message can be on a rotated index. Adds a test case to makes sure we get error info from .opendistro-anomaly-results index that has been rolled over.
kaituo
added a commit
that referenced
this pull request
May 20, 2020
* Add shingle size, total model size, and model's hash ring to profile API (#113) Hash ring helps identify node X runs the AD job for a detector Y with models on node 1,2,3. This helps oncalls locate logs. Total model size gives transparency relating to the current memory usage. What's more, shingle size help answer question "why my detector does not report anything?" This PR adds the above info to profile API via a broadcast call that consults ModelManager and FeatureManager about current state pertaining to a detector. Then these states are consolidated into information humans can parse. This PR also queries all AD result indices instead of only current result index so that we can fetch a stopped detector's error after the result index with the error is rotated. Testing done: 1. add unit tests for the newly added code 2. Run end-to-end testing to verify new profiles make senses when a detector stops running and is running * Fix bug in profile API (#115) DetectorProfile's merge does not include new fields added. This PR fixes that. Testing done: * Manually verified profile API works as expected * Improve profile API's error fetching efficiency (#117) Previously, profile API scans all anomaly result indices to get a detector's most recent error, which can cause performance bottleneck with large anomaly result indices. This PR improves this aspect via various efforts. First, when a detector is running, we only need to scan the current index, not all of the rolled over ones since we are interested in the latest error. Second, when a detector is disabled, we only need to scan the latest anomaly result indices created before the detector's enable time. Third, setting track total hits false makes ES terminate search early. ES will not try to count the number of documents and will be able to end the query as soon as N document have been collected per segment. Testing done: 1. patched a cluster with 1,000 detectors and 2GB anomaly result indices. Without the PR, scanning anomaly result indices 1000 times would timeout after 30 seconds. After the PR, we would not see the timeout. 2. A detector's error message can be on a rotated index. Adds a test case to makes sure we get error info from .opendistro-anomaly-results index that has been rolled over.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue #, if available:
#111
Description of changes:
Previously, profile API scans all anomaly result indices to get a detector's most recent error, which can cause performance bottleneck with large anomaly result indices. This PR improves this aspect via various efforts.
First, when a detector is running, we only need to scan the current index, not all of the rolled over ones since we are interested in the latest error.
Second, when a detector is disabled, we only need to scan the latest anomaly result indices created before the detector's disabled time.
Third, setting track total hits false makes ES terminate search early. ES will not try to count the number of documents and will be able to end the query as soon as N document have been collected per segment.
Testing done:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.