[helix] branch master updated: Fix unit test failure for TestTaskPerformanceMetrics. (#585)

[hu...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

hulee pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/helix.git


The following commit(s) were added to refs/heads/master by this push:
     new 7f9b9b8  Fix unit test failure for TestTaskPerformanceMetrics. (#585)
7f9b9b8 is described below

commit 7f9b9b8740bac2de0f4421fc806665133e26ca27
Author: Huizhi L <ih...@gmail.com>
AuthorDate: Thu Nov 14 14:33:25 2019 -0800

    Fix unit test failure for TestTaskPerformanceMetrics. (#585)
    
    TestTaskPerformanceMetrics fails because the thread sleeps too long and reset time window is set to 1 ms in another unit test.
    The commit fixes this failure by removing the hard coded sleep time, verifying new metric values are updated and reseting time window in testCustomizedResetInterval.
---
 .../mbeans/TestTaskPerformanceMetrics.java         | 24 ++++++++++++++--------
 .../monitoring/mbeans/TestZkClientMonitor.java     | 15 +++++++++++++-
 2 files changed, 30 insertions(+), 9 deletions(-)

diff --git a/helix-core/src/test/java/org/apache/helix/monitoring/mbeans/TestTaskPerformanceMetrics.java b/helix-core/src/test/java/org/apache/helix/monitoring/mbeans/TestTaskPerformanceMetrics.java
index 2792488..9581a41 100644
--- a/helix-core/src/test/java/org/apache/helix/monitoring/mbeans/TestTaskPerformanceMetrics.java
+++ b/helix-core/src/test/java/org/apache/helix/monitoring/mbeans/TestTaskPerformanceMetrics.java
@@ -34,6 +34,8 @@ import javax.management.ObjectInstance;
 import javax.management.ObjectName;
 import javax.management.Query;
 import javax.management.QueryExp;
+
+import org.apache.helix.TestHelper;
 import org.apache.helix.integration.manager.ClusterControllerManager;
 import org.apache.helix.integration.task.MockTask;
 import org.apache.helix.task.JobConfig;
@@ -93,13 +95,19 @@ public class TestTaskPerformanceMetrics extends TaskSynchronizedTestBase {
 
     // Confirm that there are metrics computed dynamically here and keeps increasing because jobs
     // are processed one by one
-    double oldSubmissionToStartDelay = -1L;
+    double oldSubmissionToStartDelay = 0.0d;
     double oldControllerInducedDelay = -1L;
-    for (int i = 0; i < 5; i++) {
-      // The dynamic metrics should generally be updated within 2 seconds or it would be too slow
-      Thread.sleep(2000L);
 
-      extractMetrics();
+    for (int i = 0; i < 5; i++) {
+      // Wait until new dynamic metrics are updated.
+      final double oldDelay = oldSubmissionToStartDelay;
+      TestHelper.verify(() -> {
+        extractMetrics();
+        return ((double) _beanValueMap.getOrDefault("SubmissionToScheduleDelayGauge.Mean", 0.0d))
+            > oldDelay
+            && ((double) _beanValueMap.getOrDefault("SubmissionToProcessDelayGauge.Mean", 0.0d))
+            > 0.0d;
+      }, TestHelper.WAIT_DURATION);
 
       // For SubmissionToProcessDelay, the value will stay constant because the Controller will
       // create JobContext right away most of the time
@@ -130,12 +138,12 @@ public class TestTaskPerformanceMetrics extends TaskSynchronizedTestBase {
    */
   private void extractMetrics() {
     try {
-      QueryExp exp = Query.match(Query.attr("SensorName"), Query.value("*"));
+      QueryExp exp = Query.match(Query.attr("SensorName"), Query.value(CLUSTER_NAME + ".Job.*"));
       Set<ObjectInstance> mbeans = new HashSet<>(
-          ManagementFactory.getPlatformMBeanServer().queryMBeans(new ObjectName(""), exp));
+          ManagementFactory.getPlatformMBeanServer().queryMBeans(new ObjectName("ClusterStatus:*"), exp));
       for (ObjectInstance instance : mbeans) {
         ObjectName beanName = instance.getObjectName();
-        if (instance.getClassName().contains("JobMonitor")) {
+        if (instance.getClassName().endsWith("JobMonitor")) {
           MBeanInfo info = _server.getMBeanInfo(beanName);
           MBeanAttributeInfo[] infos = info.getAttributes();
           for (MBeanAttributeInfo infoItem : infos) {
diff --git a/helix-core/src/test/java/org/apache/helix/monitoring/mbeans/TestZkClientMonitor.java b/helix-core/src/test/java/org/apache/helix/monitoring/mbeans/TestZkClientMonitor.java
index 64a02fe..6d55f5b 100644
--- a/helix-core/src/test/java/org/apache/helix/monitoring/mbeans/TestZkClientMonitor.java
+++ b/helix-core/src/test/java/org/apache/helix/monitoring/mbeans/TestZkClientMonitor.java
@@ -28,6 +28,9 @@ import java.lang.management.ManagementFactory;
 import org.testng.Assert;
 import org.testng.annotations.Test;
 
+import static org.apache.helix.SystemPropertyKeys.HELIX_MONITOR_TIME_WINDOW_LENGTH_MS;
+
+
 public class TestZkClientMonitor {
   private MBeanServer _beanServer = ManagementFactory.getPlatformMBeanServer();
 
@@ -156,7 +159,8 @@ public class TestZkClientMonitor {
   @Test
   public void testCustomizedResetInterval() throws JMException, InterruptedException {
     // Use a customized reservoir sliding length of 1 ms.
-    System.setProperty("helix.monitor.slidingTimeWindow.ms", "1");
+    String timeWindowBackup = System.getProperty(HELIX_MONITOR_TIME_WINDOW_LENGTH_MS);
+    System.setProperty(HELIX_MONITOR_TIME_WINDOW_LENGTH_MS, "1");
     final String TEST_TAG = "test_tag_x";
     final String TEST_KEY = "test_key_x";
     final String TEST_INSTANCE = "test_instance_x";
@@ -180,5 +184,14 @@ public class TestZkClientMonitor {
     Assert
         .assertEquals((long) _beanServer.getAttribute(rootName, dataPropagationLatencyGaugeAttr),
             4);
+
+    // Reset the customized reservoir sliding length.
+    // Otherwise, reservoir sliding length would be kept to 1 ms for the histogram metrics
+    // in later unit tests and cause later tests' failure.
+    if (timeWindowBackup == null) {
+      System.clearProperty(HELIX_MONITOR_TIME_WINDOW_LENGTH_MS);
+    } else {
+      System.setProperty(HELIX_MONITOR_TIME_WINDOW_LENGTH_MS, timeWindowBackup);
+    }
   }
 }