Fix another case of the profile API returns prematurely (#353)

Testing done: 1. Manually verified the issue has been fixed. 2. Reproduced the issue using an unit test and verified fix using the test.
opendistro-for-elasticsearch · Dec 31, 2020 · 21418a4 · 21418a4
1 parent bb79a0f
commit 21418a4
Show file tree

Hide file tree

Showing 2 changed files with 26 additions and 4 deletions.
diff --git a/src/main/java/com/amazon/opendistroforelasticsearch/ad/AnomalyDetectorProfileRunner.java b/src/main/java/com/amazon/opendistroforelasticsearch/ad/AnomalyDetectorProfileRunner.java
@@ -188,7 +188,6 @@ private void prepareProfile(
                             CommonErrorMessages.FAIL_FETCH_ERR_MSG + detectorId,
                             false
                         );
-
                     if (profilesToCollect.contains(DetectorProfileName.ERROR)) {
                         GetRequest getStateRequest = new GetRequest(DetectorInternalState.DETECTOR_STATE_INDEX, detectorId);
                         client.get(getStateRequest, onGetDetectorState(delegateListener, detectorId, enabledTimeMs));
@@ -459,8 +458,8 @@ private ActionListener<SearchResponse> onInittedEver(
                 processInitResponse(detector, profilesToCollect, totalUpdates, false, profileBuilder, listener);
             } else {
                 createRunningStateAndInitProgress(profilesToCollect, profileBuilder);
+                listener.onResponse(profileBuilder.build());
             }
-            listener.onResponse(profileBuilder.build());
         }, exception -> {
             if (exception instanceof IndexNotFoundException) {
                 // anomaly result index is not created yet
@@ -554,7 +553,6 @@ private void processInitResponse(
             } else {
                 long intervalMins = ((IntervalTimeConfiguration) detector.getDetectionInterval()).toDuration().toMinutes();
                 InitProgressProfile initProgress = computeInitProgressProfile(totalUpdates, intervalMins);
-
                 builder.initProgress(initProgress);
             }
         }

diff --git a/src/test/java/org/elasticsearch/search/aggregations/metrics/CardinalityProfileTests.java b/src/test/java/org/elasticsearch/search/aggregations/metrics/CardinalityProfileTests.java
@@ -33,6 +33,7 @@
 import java.util.List;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicInteger;
 
 import org.elasticsearch.action.ActionListener;
 import org.elasticsearch.action.get.GetRequest;
@@ -111,6 +112,7 @@ private void setUpMultiEntityClientGet(DetectorStatus detectorStatus, JobStatus
             } else if (request.index().equals(DetectorInternalState.DETECTOR_STATE_INDEX)) {
                 switch (errorResultStatus) {
                     case NO_ERROR:
+                        listener.onResponse(null);
                         break;
                     case NULL_POINTER_EXCEPTION:
                         GetResponse response = mock(GetResponse.class);
@@ -237,7 +239,29 @@ public void testFailGetState() throws IOException, InterruptedException {
         assertTrue(inProgressLatch.await(100, TimeUnit.SECONDS));
     }
 
-    public void testFaiConfirmInitted() throws IOException, InterruptedException {
+    public void testNoResultsNoError() throws IOException, InterruptedException {
+        setUpMultiEntityClientGet(DetectorStatus.EXIST, JobStatus.ENABLED, ErrorResultStatus.NO_ERROR);
+        setUpMultiEntityClientSearch(ADResultStatus.NO_RESULT, CardinalityStatus.NORMAL);
+        setUpProfileAction();
+
+        final AtomicInteger called = new AtomicInteger(0);
+
+        runner.profile(detector.getDetectorId(), ActionListener.wrap(response -> {
+            assertTrue(response.getInitProgress() != null);
+            called.getAndIncrement();
+        }, exception -> {
+            assertTrue("Should not reach here ", false);
+            called.getAndIncrement();
+        }), totalInitProgress);
+
+        while (called.get() == 0) {
+            Thread.sleep(100);
+        }
+        // should only call onResponse once
+        assertEquals(1, called.get());
+    }
+
+    public void testFailConfirmInitted() throws IOException, InterruptedException {
         setUpMultiEntityClientGet(DetectorStatus.EXIST, JobStatus.ENABLED, ErrorResultStatus.NO_ERROR);
         setUpMultiEntityClientSearch(ADResultStatus.EXCEPTION, CardinalityStatus.NORMAL);
         setUpProfileAction();