From 34bf6bcec0666a20d12eabd5bd0c52e32cc71507 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Przemys=C5=82aw=20Witek?= Date: Tue, 16 Jul 2019 16:15:32 +0200 Subject: [PATCH] Treat big changes in searchCount as significant and persist the document after such changes (#44413) (#44424) --- .../datafeed/DatafeedTimingStatsReporter.java | 12 ++++++- .../DatafeedTimingStatsReporterTests.java | 33 ++++++++++++++----- 2 files changed, 36 insertions(+), 9 deletions(-) diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/DatafeedTimingStatsReporter.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/DatafeedTimingStatsReporter.java index 57d06828b0b..202df616036 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/DatafeedTimingStatsReporter.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/DatafeedTimingStatsReporter.java @@ -77,10 +77,20 @@ public class DatafeedTimingStatsReporter { * Returns true if given stats objects differ from each other by more than 10% for at least one of the statistics. */ public static boolean differSignificantly(DatafeedTimingStats stats1, DatafeedTimingStats stats2) { - return differSignificantly(stats1.getTotalSearchTimeMs(), stats2.getTotalSearchTimeMs()) + return countsDifferSignificantly(stats1.getSearchCount(), stats2.getSearchCount()) + || differSignificantly(stats1.getTotalSearchTimeMs(), stats2.getTotalSearchTimeMs()) || differSignificantly(stats1.getAvgSearchTimePerBucketMs(), stats2.getAvgSearchTimePerBucketMs()); } + /** + * Returns {@code true} if one of the ratios { value1 / value2, value2 / value1 } is smaller than MIN_VALID_RATIO. + * This can be interpreted as values { value1, value2 } differing significantly from each other. + */ + private static boolean countsDifferSignificantly(long value1, long value2) { + return (((double) value2) / value1 < MIN_VALID_RATIO) + || (((double) value1) / value2 < MIN_VALID_RATIO); + } + /** * Returns {@code true} if one of the ratios { value1 / value2, value2 / value1 } is smaller than MIN_VALID_RATIO or * the absolute difference |value1 - value2| is greater than MAX_VALID_ABS_DIFFERENCE_MS. diff --git a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/datafeed/DatafeedTimingStatsReporterTests.java b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/datafeed/DatafeedTimingStatsReporterTests.java index 93df704deae..e0aa9a696cd 100644 --- a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/datafeed/DatafeedTimingStatsReporterTests.java +++ b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/datafeed/DatafeedTimingStatsReporterTests.java @@ -18,6 +18,7 @@ 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; @@ -44,28 +45,40 @@ public class DatafeedTimingStatsReporterTests extends ESTestCase { verifyZeroInteractions(jobResultsPersister); } + public void testReportSearchDuration_Zero() { + DatafeedTimingStatsReporter timingStatsReporter = + new DatafeedTimingStatsReporter(new DatafeedTimingStats(JOB_ID), jobResultsPersister); + assertThat(timingStatsReporter.getCurrentTimingStats(), equalTo(new DatafeedTimingStats(JOB_ID, 0, 0, 0.0))); + + timingStatsReporter.reportSearchDuration(TimeValue.ZERO); + assertThat(timingStatsReporter.getCurrentTimingStats(), equalTo(new DatafeedTimingStats(JOB_ID, 1, 0, 0.0))); + + verify(jobResultsPersister).persistDatafeedTimingStats(new DatafeedTimingStats(JOB_ID, 1, 0, 0.0), RefreshPolicy.IMMEDIATE); + verifyNoMoreInteractions(jobResultsPersister); + } + public void testReportSearchDuration() { DatafeedTimingStatsReporter timingStatsReporter = - new DatafeedTimingStatsReporter(new DatafeedTimingStats(JOB_ID, 3, 10, 10000.0), jobResultsPersister); - assertThat(timingStatsReporter.getCurrentTimingStats(), equalTo(new DatafeedTimingStats(JOB_ID, 3, 10, 10000.0))); + new DatafeedTimingStatsReporter(new DatafeedTimingStats(JOB_ID, 13, 10, 10000.0), jobResultsPersister); + assertThat(timingStatsReporter.getCurrentTimingStats(), equalTo(new DatafeedTimingStats(JOB_ID, 13, 10, 10000.0))); timingStatsReporter.reportSearchDuration(ONE_SECOND); - assertThat(timingStatsReporter.getCurrentTimingStats(), equalTo(new DatafeedTimingStats(JOB_ID, 4, 10, 11000.0))); + assertThat(timingStatsReporter.getCurrentTimingStats(), equalTo(new DatafeedTimingStats(JOB_ID, 14, 10, 11000.0))); timingStatsReporter.reportSearchDuration(ONE_SECOND); - assertThat(timingStatsReporter.getCurrentTimingStats(), equalTo(new DatafeedTimingStats(JOB_ID, 5, 10, 12000.0))); + assertThat(timingStatsReporter.getCurrentTimingStats(), equalTo(new DatafeedTimingStats(JOB_ID, 15, 10, 12000.0))); timingStatsReporter.reportSearchDuration(ONE_SECOND); - assertThat(timingStatsReporter.getCurrentTimingStats(), equalTo(new DatafeedTimingStats(JOB_ID, 6, 10, 13000.0))); + assertThat(timingStatsReporter.getCurrentTimingStats(), equalTo(new DatafeedTimingStats(JOB_ID, 16, 10, 13000.0))); timingStatsReporter.reportSearchDuration(ONE_SECOND); - assertThat(timingStatsReporter.getCurrentTimingStats(), equalTo(new DatafeedTimingStats(JOB_ID, 7, 10, 14000.0))); + assertThat(timingStatsReporter.getCurrentTimingStats(), equalTo(new DatafeedTimingStats(JOB_ID, 17, 10, 14000.0))); InOrder inOrder = inOrder(jobResultsPersister); inOrder.verify(jobResultsPersister).persistDatafeedTimingStats( - new DatafeedTimingStats(JOB_ID, 5, 10, 12000.0), RefreshPolicy.IMMEDIATE); + new DatafeedTimingStats(JOB_ID, 15, 10, 12000.0), RefreshPolicy.IMMEDIATE); inOrder.verify(jobResultsPersister).persistDatafeedTimingStats( - new DatafeedTimingStats(JOB_ID, 7, 10, 14000.0), RefreshPolicy.IMMEDIATE); + new DatafeedTimingStats(JOB_ID, 17, 10, 14000.0), RefreshPolicy.IMMEDIATE); verifyNoMoreInteractions(jobResultsPersister); } @@ -134,5 +147,9 @@ public class DatafeedTimingStatsReporterTests extends ESTestCase { DatafeedTimingStatsReporter.differSignificantly( new DatafeedTimingStats(JOB_ID, 5, 10, 100000.0), new DatafeedTimingStats(JOB_ID, 5, 10, 110001.0)), is(true)); + assertThat( + DatafeedTimingStatsReporter.differSignificantly( + new DatafeedTimingStats(JOB_ID, 5, 10, 100000.0), new DatafeedTimingStats(JOB_ID, 50, 10, 100000.0)), + is(true)); } }