Fix empty preview result due to insufficient sample #65
Conversation
Preview does not use all data in the given time range as it is costly. Previously, we sample data by issuing multiple queries on shard 0 data. The purpose of the shard 0 query restriction is to reduce system costs. The nab_art_daily_jumpsup data set has one doc in each interval, and the doc is spread out in 5 shards. Even though we issue 360 queries, we only get 70~80 samples back by querying shard 0. Together with interpolated data points, the preview run misses significant portions of data required to train models (400 is the minimum) and thus returns empty preview results. This PR fixes the issue by removing the shard 0 search restriction. Previously, the preview API issues multiple queries encapsulated in a multisearch request (the request can contain 360 search queries at most). The same result could be obtained via a date range query with multiple range buckets. We show a date range query is 2~10 times faster than a multisearch request (opendistro-for-elasticsearch#63). This PR replaces the multisearch request with a date range query. This PR also removes unused field scriptService in SearchFeatureDao. Testing done: - Previous preview unit tests pass. - Manually verified date range queries results are correctly processed by cross checking intermediate logs. - Manually verified preview results with multisearch and date range implementation are the same. - Manually verified preview don't show empty results with the nab_art_daily_jumpsup data set with the fix
src/main/java/com/amazon/opendistroforelasticsearch/ad/feature/SearchFeatureDao.java
Show resolved
Hide resolved
src/main/java/com/amazon/opendistroforelasticsearch/ad/feature/SearchFeatureDao.java
Show resolved
Hide resolved
src/main/java/com/amazon/opendistroforelasticsearch/ad/feature/SearchFeatureDao.java
Outdated
Show resolved
Hide resolved
src/main/java/com/amazon/opendistroforelasticsearch/ad/feature/SearchFeatureDao.java
Outdated
Show resolved
Hide resolved
src/main/java/com/amazon/opendistroforelasticsearch/ad/feature/SearchFeatureDao.java
Outdated
Show resolved
Hide resolved
| aggs | ||
| .asList() | ||
| .stream() | ||
| .filter(InternalDateRange.class::isInstance) |
There was a problem hiding this comment.
Is it possible ES returns different date range class other than InternalDateRange for this case?
There was a problem hiding this comment.
for 7.4, it is not possible. For other versions, it is possible.
There was a problem hiding this comment.
Got it. So for other version, the InternalDateRange.class is still what we want?
There was a problem hiding this comment.
other versions is untested. Will do that when we release new versions.
src/main/java/com/amazon/opendistroforelasticsearch/ad/feature/SearchFeatureDao.java
Show resolved
Hide resolved
| @@ -336,4 +311,30 @@ private SearchRequest createFeatureSearchRequest(AnomalyDetector detector, long | |||
| throw new IllegalStateException(e); | |||
There was a problem hiding this comment.
same as Yaliang's comment.
There was a problem hiding this comment.
This is Lai's code I guess. I guess IOException is checked exception and the method and and its callers have to declare the exception before the exception is handled. Lai prefers to use unchecked exception like IllegalStateException.
I prefer checked exceptions as they serve as documentation on what a method can throw and remind the caller to handle it.
Do you have any preference on when to use checked and unchecked exceptions?
| logger.error("Unexpected error running anomaly detector " + detector.getDetectorId(), exception); | ||
| try { | ||
| XContentBuilder builder = channel.newBuilder().startObject().field(ANOMALY_DETECTOR, detector).endObject(); | ||
| channel.sendResponse(new BytesRestResponse(RestStatus.OK, builder)); |
There was a problem hiding this comment.
question: why send OK response if there is an exception while running AD?
There was a problem hiding this comment.
Good catch. I changed to RestStatus.INTERNAL_SERVER_ERROR.
Issue #, if available:
#64
Description of changes:
Preview does not use all data in the given time range as it is costly. Previously, we sample data by issuing multiple queries on shard 0 data. The purpose of the shard 0 query restriction is to reduce system costs. The nab_art_daily_jumpsup data set has one doc in each interval, and the doc is spread out in 5 shards. Even though we issue 360 queries, we only get 70~80 samples back by querying shard 0. Together with interpolated data points, the preview run misses significant portions of data required to train models (400 is the minimum) and thus returns empty preview results. This PR fixes the issue by removing the shard 0 search restriction.
Previously, the preview API issues multiple queries encapsulated in a multisearch request (the request can contain 360 search queries at most). The same result could be obtained via a date range query with multiple range buckets. We show a date range query is 2~10 times faster than a multisearch request (#63). This PR replaces the multisearch request with a date range query.
This PR also fixes a bug in query generation. We generate aggregation query twice: once with filter query, once separately.
This PR also removes unused field scriptService in SearchFeatureDao.
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.