Fix flaky test ResourceAwareTasksTests on windows. (opensearch-projec…

…t#5077) (opensearch-project#5078) Signed-off-by: Marc Handalian <[email protected]> Add Changelog entry. Signed-off-by: Marc Handalian <[email protected]> Revert unintentional removal of memory assertion. Signed-off-by: Marc Handalian <[email protected]> Signed-off-by: Marc Handalian <[email protected]> Signed-off-by: Marc Handalian <[email protected]> Co-authored-by: Marc Handalian <[email protected]>
mch2 · Nov 4, 2022 · 07e9111 · 07e9111
1 parent d1b4340
commit 07e9111
Show file tree

Hide file tree

Showing 2 changed files with 17 additions and 4 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -113,6 +113,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 - Fix for failing checkExtraction, checkLicense and checkNotice tasks for windows gradle check ([#4941](https://github.com/opensearch-project/OpenSearch/pull/4941))
 - [BUG]: Allow decommission to support delay timeout [#4930](https://github.com/opensearch-project/OpenSearch/pull/4930))
 - Fixed misunderstanding message "No OpenSearchException found" when detailed_error disabled by return meaningful messages ([#4708](https://github.com/opensearch-project/OpenSearch/pull/4708))
+- Fix flaky test ResourceAwareTasksTests on Windows ([#5077](https://github.com/opensearch-project/OpenSearch/pull/5077))
 
 ### Security
 - CVE-2022-25857 org.yaml:snakeyaml DOS vulnerability ([#4341](https://github.com/opensearch-project/OpenSearch/pull/4341))

diff --git a/...src/test/java/org/opensearch/action/admin/cluster/node/tasks/ResourceAwareTasksTests.java b/...src/test/java/org/opensearch/action/admin/cluster/node/tasks/ResourceAwareTasksTests.java
@@ -9,6 +9,7 @@
 package org.opensearch.action.admin.cluster.node.tasks;
 
 import com.sun.management.ThreadMXBean;
+import org.apache.lucene.util.Constants;
 import org.opensearch.ExceptionsHelper;
 import org.opensearch.action.ActionListener;
 import org.opensearch.action.NotifyOnceListener;
@@ -303,7 +304,7 @@ public void testBasicTaskResourceTracking() throws Exception {
                 actualTaskMemoryOverhead - taskTestContext.memoryConsumptionWhenExecutionStarts,
                 expectedArrayAllocationOverhead
             );
-            assertTrue(task.getTotalResourceStats().getCpuTimeInNanos() > 0);
+            assertCPUTime(task.getTotalResourceStats().getCpuTimeInNanos());
         };
 
         startResourceAwareNodesAction(testNodes[0], false, taskTestContext, new ActionListener<NodesResponse>() {
@@ -362,7 +363,7 @@ public void testTaskResourceTrackingDuringTaskCancellation() throws Exception {
                 actualTaskMemoryOverhead - taskTestContext.memoryConsumptionWhenExecutionStarts,
                 expectedOverhead
             );
-            assertTrue(task.getTotalResourceStats().getCpuTimeInNanos() > 0);
+            assertCPUTime(task.getTotalResourceStats().getCpuTimeInNanos());
         };
 
         startResourceAwareNodesAction(testNodes[0], true, taskTestContext, new ActionListener<NodesResponse>() {
@@ -463,7 +464,7 @@ public void testTaskResourceTrackingDisabledWhileTaskInProgress() throws Excepti
                 actualTaskMemoryOverhead - taskTestContext.memoryConsumptionWhenExecutionStarts,
                 expectedArrayAllocationOverhead
             );
-            assertTrue(task.getTotalResourceStats().getCpuTimeInNanos() > 0);
+            assertCPUTime(task.getTotalResourceStats().getCpuTimeInNanos());
         };
 
         startResourceAwareNodesAction(testNodes[0], false, taskTestContext, new ActionListener<NodesResponse>() {
@@ -543,7 +544,7 @@ public void testOnDemandRefreshWhileFetchingTasks() throws InterruptedException
 
             assertNotNull(taskInfo.getResourceStats());
             assertNotNull(taskInfo.getResourceStats().getResourceUsageInfo());
-            assertTrue(taskInfo.getResourceStats().getResourceUsageInfo().get("total").getCpuTimeInNanos() > 0);
+            assertCPUTime(taskInfo.getResourceStats().getResourceUsageInfo().get("total").getCpuTimeInNanos());
             assertTrue(taskInfo.getResourceStats().getResourceUsageInfo().get("total").getMemoryInBytes() > 0);
         };
 
@@ -655,4 +656,15 @@ private void assertMemoryUsageWithinLimits(long actual, long expected) {
         long maxOverhead = Math.min(200000, expected * 5 / 100);
         assertThat(actual, lessThanOrEqualTo(expected + maxOverhead));
     }
+
+    private void assertCPUTime(long cpuTimeInNanos) {
+        // Windows registers a cpu tick at a default of ~15ms which is slightly slower than other OSs.
+        // The work done within the runnable in this test often completes in under that time and returns a 0 value from
+        // ThreadMXBean.getThreadCpuTime. To reduce flakiness in this test accept 0 as a value on Windows.
+        if (Constants.WINDOWS) {
+            assertTrue("Cpu should be non negative on windows", cpuTimeInNanos >= 0);
+        } else {
+            assertTrue("Cpu should have a positive value", cpuTimeInNanos > 0);
+        }
+    }
 }