don't throw exception if no result indices found when search both def…

…ault and custom result index (#322) * don't throw exception if no result indices found when search both default and custom result index Signed-off-by: Yaliang Wu <[email protected]> * add more comments Signed-off-by: Yaliang Wu <[email protected]> * fix issue link Signed-off-by: Yaliang Wu <[email protected]>
opensearch-project · Nov 23, 2021 · 2c61d6d · 2c61d6d
1 parent 9b37290
commit 2c61d6d
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 14 deletions.
diff --git a/src/main/java/org/opensearch/ad/transport/SearchAnomalyResultTransportAction.java b/src/main/java/org/opensearch/ad/transport/SearchAnomalyResultTransportAction.java
@@ -90,19 +90,28 @@ protected void doExecute(Task task, SearchRequest request, ActionListener<Search
 
         String[] concreteIndices = indexNameExpressionResolver
             .concreteIndexNames(clusterService.state(), IndicesOptions.lenientExpandOpen(), indices);
-        if (concreteIndices == null || concreteIndices.length == 0) {
-            // No result indices found, will throw exception
-            listener.onFailure(new IllegalArgumentException("No indices found"));
-            return;
-        }
+        // If concreteIndices is null or empty, don't throw exception. Detector list page will search both
+        // default and custom result indices to get anomaly of last 24 hours. If throw exception, detector
+        // list page will throw error and won't show any detector.
+        // If a cluster has no custom result indices, and some new non-custom-result-detector that hasn't
+        // finished one interval (where no default result index exists), then no result indices found. We
+        // will still search ".opendistro-anomaly-results*" (even these default indices don't exist) to
+        // return an empty SearchResponse. This search looks unnecessary, but this can make sure the
+        // detector list page show all detectors correctly. The other solution is to catch errors from
+        // frontend when search anomaly results to make sure frontend won't crash. Check this Github issue:
+        // https://github.com/opensearch-project/anomaly-detection-dashboards-plugin/issues/154
 
         Set<String> customResultIndices = new HashSet<>();
-        for (String index : concreteIndices) {
-            if (index.startsWith(CUSTOM_RESULT_INDEX_PREFIX)) {
-                customResultIndices.add(index);
+        if (concreteIndices != null) {
+            for (String index : concreteIndices) {
+                if (index.startsWith(CUSTOM_RESULT_INDEX_PREFIX)) {
+                    customResultIndices.add(index);
+                }
             }
         }
 
+        // If user need to query custom result index only, and that custom result index deleted. Then
+        // we should not search anymore. Just throw exception here.
         if (onlyQueryCustomResultIndex && customResultIndices.size() == 0) {
             listener.onFailure(new IllegalArgumentException("No custom result indices found"));
             return;

diff --git a/src/test/java/org/opensearch/ad/transport/SearchAnomalyResultActionTests.java b/src/test/java/org/opensearch/ad/transport/SearchAnomalyResultActionTests.java
@@ -40,12 +40,10 @@ public void testSearchResultAction() throws IOException {
     @Test
     public void testNoIndex() {
         deleteIndexIfExists(CommonName.ANOMALY_RESULT_INDEX_ALIAS);
-        expectThrows(
-            IllegalArgumentException.class,
-            () -> client()
-                .execute(SearchAnomalyResultAction.INSTANCE, matchAllRequest().indices(ALL_AD_RESULTS_INDEX_PATTERN))
-                .actionGet(10000)
-        );
+        SearchResponse searchResponse = client()
+            .execute(SearchAnomalyResultAction.INSTANCE, matchAllRequest().indices(ALL_AD_RESULTS_INDEX_PATTERN))
+            .actionGet(10000);
+        assertEquals(0, searchResponse.getHits().getTotalHits().value);
     }
 
 }