Fix flaky testDisableErrorLogByDefault (#2869)

Fix flaky testDisableErrorLogByDefault, update StatusUpdateUtil
apache · Aug 6, 2024 · bb29cec · bb29cec
1 parent 2add0c4
commit bb29cec
Show file tree

Hide file tree

Showing 3 changed files with 24 additions and 31 deletions.
diff --git a/helix-core/src/main/java/org/apache/helix/util/StatusUpdateUtil.java b/helix-core/src/main/java/org/apache/helix/util/StatusUpdateUtil.java
@@ -555,9 +555,16 @@ void publishStatusUpdateRecord(ZNRecord record, Message message, Level level,
    */
   void publishErrorRecord(ZNRecord record, String instanceName, String updateSubPath,
       String updateKey, String sessionId, HelixDataAccessor accessor, boolean isController) {
-    if (!ERROR_LOG_TO_ZK_ENABLED) {
+    publishErrorRecord(record, instanceName, updateSubPath, updateKey, sessionId, accessor,
+        isController, ERROR_LOG_TO_ZK_ENABLED);
+  }
+
+  void publishErrorRecord(ZNRecord record, String instanceName, String updateSubPath,
+      String updateKey, String sessionId, HelixDataAccessor accessor, boolean isController, boolean logToZKEnabled) {
+    if (!logToZKEnabled) {
       return;
     }
+
     Builder keyBuilder = accessor.keyBuilder();
     if (isController) {
       // TODO need to fix: ERRORS_CONTROLLER doesn't have a form of

diff --git a/helix-core/src/test/java/org/apache/helix/integration/manager/TestParticipantManager.java b/helix-core/src/test/java/org/apache/helix/integration/manager/TestParticipantManager.java
@@ -44,7 +44,6 @@
 import org.apache.helix.SystemPropertyKeys;
 import org.apache.helix.TestHelper;
 import org.apache.helix.model.ClusterConfig;
-import org.apache.helix.model.InstanceConfig;
 import org.apache.helix.model.ParticipantHistory;
 import org.apache.helix.zookeeper.datamodel.ZNRecord;
 import org.apache.helix.ZkTestHelper;
@@ -78,16 +77,16 @@ public class TestParticipantManager extends ZkTestBase {
     System.setProperty(SystemPropertyKeys.STATEUPDATEUTIL_ERROR_PERSISTENCY_ENABLED, "true");
   }
 
-  @AfterMethod
-  public void afterMethod(Method testMethod, ITestContext testContext) {
-    deleteCluster(_clusterName);
-  }
-
   @AfterClass
   public void afterClass() {
     System.clearProperty(SystemPropertyKeys.STATEUPDATEUTIL_ERROR_PERSISTENCY_ENABLED);
   }
 
+  @AfterMethod
+  public void afterMethod(Method testMethod, ITestContext testContext) {
+    deleteCluster(_clusterName);
+  }
+
   @Test
   public void simpleIntegrationTest() throws Exception {
     TestHelper.setupCluster(_clusterName, ZK_ADDR, 12918, // participant port

diff --git a/helix-core/src/test/java/org/apache/helix/util/TestStatusUpdateUtil.java b/helix-core/src/test/java/org/apache/helix/util/TestStatusUpdateUtil.java
@@ -24,13 +24,11 @@
 
 import org.apache.helix.HelixConstants;
 import org.apache.helix.PropertyPathBuilder;
-import org.apache.helix.SystemPropertyKeys;
 import org.apache.helix.TestHelper;
 import org.apache.helix.common.ZkTestBase;
 import org.apache.helix.integration.manager.MockParticipantManager;
 import org.apache.helix.messaging.handling.HelixStateTransitionHandler;
 import org.apache.helix.model.Message;
-import org.apache.helix.model.StatusUpdate;
 import org.apache.helix.zookeeper.datamodel.ZNRecord;
 import org.apache.helix.zookeeper.zkclient.exception.ZkException;
 import org.testng.Assert;
@@ -45,17 +43,6 @@ public class TestStatusUpdateUtil extends ZkTestBase {
   private Message message = new Message(Message.MessageType.STATE_TRANSITION, "Some unique id");
   private MockParticipantManager[] participants = new MockParticipantManager[n];
 
-
-  static void setFinalStatic(Field field, Object newValue) throws Exception {
-    field.setAccessible(true);
-
-    Field modifiersField = Field.class.getDeclaredField("modifiers");
-    modifiersField.setAccessible(true);
-    modifiersField.setInt(field, field.getModifiers() & ~Modifier.FINAL);
-
-    field.set(null, newValue);
-  }
-
   @AfterClass
   public void afterClass() {
     for (int i = 0; i < n; i++) {
@@ -98,15 +85,16 @@ public void beforeClass() throws Exception {
   @Test(dependsOnMethods = "testDisableErrorLogByDefault")
   public void testEnableErrorLog() throws Exception {
     StatusUpdateUtil statusUpdateUtil = new StatusUpdateUtil();
-    setFinalStatic(StatusUpdateUtil.class.getField("ERROR_LOG_TO_ZK_ENABLED"), true);
 
-    Exception e = new RuntimeException("test exception");
-    statusUpdateUtil.logError(message, HelixStateTransitionHandler.class, e,
-        "test status update", participants[0]);
-    // logged to Zookeeper
+
     String errPath = PropertyPathBuilder
         .instanceError(clusterName, "localhost_12918", participants[0].getSessionId(), "TestDB",
             "TestDB_0");
+    ZNRecord messageRecord = statusUpdateUtil.createMessageStatusUpdateRecord(message, StatusUpdateUtil.Level.HELIX_ERROR, HelixStateTransitionHandler.class,
+        "testEnableErrorLog");
+    // logged to Zookeeper
+    statusUpdateUtil.publishErrorRecord(messageRecord, participants[0].getInstanceName(), message.getResourceName(), message.getPartitionName(), participants[0].getSessionId(),
+        participants[0].getHelixDataAccessor(), false, true);
 
     try {
       ZNRecord error = _gZkClient.readData(errPath);
@@ -118,16 +106,15 @@ public void testEnableErrorLog() throws Exception {
   @Test
   public void testDisableErrorLogByDefault() throws Exception {
     StatusUpdateUtil statusUpdateUtil = new StatusUpdateUtil();
-    setFinalStatic(StatusUpdateUtil.class.getField("ERROR_LOG_TO_ZK_ENABLED"), false);
-
-    Exception e = new RuntimeException("test exception");
-    statusUpdateUtil.logError(message, HelixStateTransitionHandler.class, e,
-        "test status update", participants[0]);
-
+    ZNRecord messageRecord = statusUpdateUtil.createMessageStatusUpdateRecord(message, StatusUpdateUtil.Level.HELIX_ERROR, HelixStateTransitionHandler.class,
+        "testDisableErrorLogByDefault");
+    statusUpdateUtil.publishErrorRecord(messageRecord, participants[0].getInstanceName(), message.getResourceName(), message.getPartitionName(), participants[0].getSessionId(),
+        participants[0].getHelixDataAccessor(), false, false);
     // assert by default, not logged to Zookeeper
     String errPath = PropertyPathBuilder
         .instanceError(clusterName, "localhost_12918", participants[0].getSessionId(), "TestDB",
             "TestDB_0");
+
     try {
       ZNRecord error = _gZkClient.readData(errPath);
       Assert.fail(String.format(