Skip to content
This repository has been archived by the owner on Aug 2, 2022. It is now read-only.

Cancel query code path needs to be updated #189

Open
kaituo opened this issue Jul 16, 2020 · 0 comments
Open

Cancel query code path needs to be updated #189

kaituo opened this issue Jul 16, 2020 · 0 comments
Labels
enhancement New feature or request

Comments

@kaituo
Copy link
Member

kaituo commented Jul 16, 2020

Is your feature request related to a problem? Please describe.
We don't guarantee there is one query running for each detector any more. This is the old change #54. Since then, we have moved away from the old code path.

Describe the solution you'd like
Hanguang’s change only covers throttledTimeRequest: https://github.com/opendistro-for-elasticsearch/anomaly-detection/blob/master/src/main/java/com/amazon/opendistroforelasticsearch/ad/feature/SearchFeatureDao.java#L153

Now getFeaturesForPeriod is deprecated: https://github.com/opendistro-for-elasticsearch/anomaly-detection/blob/master/src/main/java/com/amazon/opendistroforelasticsearch/ad/feature/SearchFeatureDao.java#L147

We use https://github.com/opendistro-for-elasticsearch/anomaly-detection/blob/master/src/main/java/com/amazon/opendistroforelasticsearch/ad/feature/SearchFeatureDao.java#L165 and https://github.com/opendistro-for-elasticsearch/anomaly-detection/blob/master/src/main/java/com/amazon/opendistroforelasticsearch/ad/feature/SearchFeatureDao.java#L202

We want to change https://github.com/opendistro-for-elasticsearch/anomaly-detection/blob/master/src/main/java/com/amazon/opendistroforelasticsearch/ad/feature/SearchFeatureDao.java#L168 and https://github.com/opendistro-for-elasticsearch/anomaly-detection/blob/master/src/main/java/com/amazon/opendistroforelasticsearch/ad/feature/SearchFeatureDao.java#L209 so that we use https://github.com/opendistro-for-elasticsearch/anomaly-detection/blob/master/src/main/java/com/amazon/opendistroforelasticsearch/ad/util/ClientUtil.java#L125

https://github.com/opendistro-for-elasticsearch/anomaly-detection/blob/master/src/main/java/com/amazon/opendistroforelasticsearch/ad/feature/SearchFeatureDao.java#L202 is used in each detector run and cold start. We don't want to cancel query if it is for cold start since it is one time thing. We probably want to add a parameter to getFeatureSamplesForPeriods to indicate if the query should be cancelled or not. Say if it is used for cold start, then the parameter is false.

And add cacnel query mechanism in https://github.com/opendistro-for-elasticsearch/anomaly-detection/blob/master/src/main/java/com/amazon/opendistroforelasticsearch/ad/util/ClientUtil.java#L125 similar to https://github.com/opendistro-for-elasticsearch/anomaly-detection/blob/master/src/main/java/com/amazon/opendistroforelasticsearch/ad/util/ClientUtil.java#L85-L115

Also, we want to add an ESIntegTestCase where we

  1. define a SearchOperationListener such that index operations are delayed to simulate long running queries;
  2. create a fake plugin to use listener defined in 1)
  3. add AD plugin and the fake plugin together
  4. ... automate what we did on manual testing ..

so 3 parts of the change:

  1. modify current SearchFeatureDao.getFeaturesForPeriod and SearchFeatureDao.getFeatureSamplesForPeriodsto use ClientUtil.asyncRequest and add a parameter to indicate if the query is cancellable or not.
  2. add cancel mechanism in ClientUtil.asyncRequest similar to Hanguang's PR.
  3. write an integ test
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants