From bb29cece3b78a4f0283f44e3b3362837c0c6eaa7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Grant=20Pal=C3=A1u=20Spencer?= Date: Tue, 6 Aug 2024 11:26:01 -0700 Subject: [PATCH] Fix flaky testDisableErrorLogByDefault (#2869) Fix flaky testDisableErrorLogByDefault, update StatusUpdateUtil --- .../apache/helix/util/StatusUpdateUtil.java | 9 ++++- .../manager/TestParticipantManager.java | 11 +++--- .../helix/util/TestStatusUpdateUtil.java | 35 ++++++------------- 3 files changed, 24 insertions(+), 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 index 55b6dfef98..75185bbeb6 100644 --- 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 index ed23a05f9a..b198deeb57 100644 --- 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 index 84f4397219..9a0a3686e8 100644 --- 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(