Small improvements

elastic · Jun 27, 2019 · 2617de0 · 2617de0
1 parent e8c0116
commit 2617de0
Show file tree

Hide file tree

Showing 3 changed files with 16 additions and 15 deletions.
diff --git a/...ugin/core/src/main/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedTimingStats.java b/...ugin/core/src/main/java/org/elasticsearch/xpack/core/ml/datafeed/DatafeedTimingStats.java
@@ -72,8 +72,8 @@ public double getTotalSearchTimeMs() {
         return totalSearchTimeMs;
     }
 
-    public void incrementTotalSearchTimeMs(double totalSearchTimeMs) {
-        this.totalSearchTimeMs += totalSearchTimeMs;
+    public void incrementTotalSearchTimeMs(double searchTimeMs) {
+        this.totalSearchTimeMs += searchTimeMs;
     }
 
     @Override

diff --git a/...gin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/DatafeedTimingStatsReporter.java b/...gin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/DatafeedTimingStatsReporter.java
@@ -49,12 +49,15 @@ private void reportSearchDuration(Duration searchDuration) {
         double searchDurationMs = searchDuration.toMillis();
         currentTimingStats.incrementTotalSearchTimeMs(searchDurationMs);
         if (differSignificantly(currentTimingStats, persistedTimingStats)) {
-            jobResultsPersister.persistDatafeedTimingStats(currentTimingStats);
-            persistedTimingStats = currentTimingStats;
-            currentTimingStats = new DatafeedTimingStats(persistedTimingStats);
+            flush();
         }
     }
 
+    private void flush() {
+        persistedTimingStats = new DatafeedTimingStats(currentTimingStats);
+        jobResultsPersister.persistDatafeedTimingStats(persistedTimingStats);
+    }
+
     /**
      * Returns true if given stats objects differ from each other by more than 10% for at least one of the statistics.
      */
@@ -82,5 +85,5 @@ private static boolean differSignificantly(double value1, double value2) {
      * Maximum absolute difference of values that is interpreted as values being similar.
      * If the values absolute difference is greater than MAX_VALID_ABS_DIFFERENCE, the values are interpreted as significantly different.
      */
-    private static final double MAX_VALID_ABS_DIFFERENCE_MS = 10000.0;
+    private static final double MAX_VALID_ABS_DIFFERENCE_MS = 10000.0;  // 10s
 }
diff --git a/...l/src/test/java/org/elasticsearch/xpack/ml/datafeed/DatafeedTimingStatsReporterTests.java b/...l/src/test/java/org/elasticsearch/xpack/ml/datafeed/DatafeedTimingStatsReporterTests.java
@@ -9,6 +9,7 @@
 import org.elasticsearch.xpack.core.ml.datafeed.DatafeedTimingStats;
 import org.elasticsearch.xpack.ml.job.persistence.JobResultsPersister;
 import org.junit.Before;
+import org.mockito.InOrder;
 
 import java.time.Clock;
 import java.time.Duration;
@@ -17,10 +18,8 @@
 
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.is;
+import static org.mockito.Mockito.inOrder;
 import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.verify;
-import static org.mockito.Mockito.verifyNoMoreInteractions;
-import static org.mockito.Mockito.verifyZeroInteractions;
 
 public class DatafeedTimingStatsReporterTests extends ESTestCase {
 
@@ -43,21 +42,20 @@ public void testExecuteWithReporting() {
 
         timingStatsReporter.executeWithReporting(clock::advanceTime, ONE_SECOND);
         assertThat(timingStatsReporter.getCurrentTimingStats(), equalTo(new DatafeedTimingStats(JOB_ID, 11000.0)));
-        verifyZeroInteractions(jobResultsPersister);
 
         timingStatsReporter.executeWithReporting(clock::advanceTime, ONE_SECOND);
         assertThat(timingStatsReporter.getCurrentTimingStats(), equalTo(new DatafeedTimingStats(JOB_ID, 12000.0)));
-        verify(jobResultsPersister).persistDatafeedTimingStats(new DatafeedTimingStats(JOB_ID, 12000.0));
-        verifyNoMoreInteractions(jobResultsPersister);
 
         timingStatsReporter.executeWithReporting(clock::advanceTime, ONE_SECOND);
         assertThat(timingStatsReporter.getCurrentTimingStats(), equalTo(new DatafeedTimingStats(JOB_ID, 13000.0)));
-        verifyZeroInteractions(jobResultsPersister);
 
         timingStatsReporter.executeWithReporting(clock::advanceTime, ONE_SECOND);
         assertThat(timingStatsReporter.getCurrentTimingStats(), equalTo(new DatafeedTimingStats(JOB_ID, 14000.0)));
-        verify(jobResultsPersister).persistDatafeedTimingStats(new DatafeedTimingStats(JOB_ID, 14000.0));
-        verifyNoMoreInteractions(jobResultsPersister);
+
+        InOrder inOrder = inOrder(jobResultsPersister);
+        inOrder.verify(jobResultsPersister).persistDatafeedTimingStats(new DatafeedTimingStats(JOB_ID, 12000.0));
+        inOrder.verify(jobResultsPersister).persistDatafeedTimingStats(new DatafeedTimingStats(JOB_ID, 14000.0));
+        inOrder.verifyNoMoreInteractions();
     }
 
     /** Mutable clock that allows advancing current time. */