From 0a342bc4e027f90ec95797fa42c89c5983d7fd09 Mon Sep 17 00:00:00 2001 From: Marc Handalian Date: Fri, 4 Nov 2022 09:02:56 -0700 Subject: [PATCH] Fix flaky test ResourceAwareTasksTests on windows. Signed-off-by: Marc Handalian Add Changelog entry. Signed-off-by: Marc Handalian Revert unintentional removal of memory assertion. Signed-off-by: Marc Handalian --- CHANGELOG.md | 1 + .../node/tasks/ResourceAwareTasksTests.java | 20 +++++++++++++++---- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 05ec47bce635d..cb3fcee667836 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -206,6 +206,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - Backport failures for merge conflicts on CHANGELOG.md file ([#4977](https://github.com/opensearch-project/OpenSearch/pull/4977)) - Remove gradle-check dependency on precommit [#5027](https://github.com/opensearch-project/OpenSearch/pull/5027) - Fix version check for 2.x release for awareness attribute decommission([#5034](https://github.com/opensearch-project/OpenSearch/pull/5034)) +- 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/server/src/test/java/org/opensearch/action/admin/cluster/node/tasks/ResourceAwareTasksTests.java b/server/src/test/java/org/opensearch/action/admin/cluster/node/tasks/ResourceAwareTasksTests.java index d49dd14492327..8b0dd8fc5cf78 100644 --- a/server/src/test/java/org/opensearch/action/admin/cluster/node/tasks/ResourceAwareTasksTests.java +++ b/server/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() { @@ -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() { @@ -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() { @@ -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); + } + } }