From c0097fab1f21c28c5f957c714fc942bbee2dee06 Mon Sep 17 00:00:00 2001 From: Sreeja Chintalapati Date: Wed, 12 Feb 2025 10:12:05 +0530 Subject: [PATCH 1/4] HDDS-12179. Improve Container Log to also capture the Command fired to Datanode --- .../container/common/utils/ContainerLogger.java | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/utils/ContainerLogger.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/utils/ContainerLogger.java index 6a5c5c117588..2447f38808dc 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/utils/ContainerLogger.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/utils/ContainerLogger.java @@ -51,7 +51,7 @@ private ContainerLogger() { } * @param containerData The container that was created and opened. */ public static void logOpen(ContainerData containerData) { - LOG.info(getMessage(containerData)); + LOG.info(getMessage(containerData,"CommandType=CreateContainer")); } /** @@ -80,7 +80,7 @@ public static void logQuasiClosed(ContainerData containerData, * @param containerData The container that was closed. */ public static void logClosed(ContainerData containerData) { - LOG.info(getMessage(containerData)); + LOG.info(getMessage(containerData,"CommandType=CloseContainer")); } /** @@ -116,9 +116,9 @@ public static void logLost(ContainerData containerData, String message) { */ public static void logDeleted(ContainerData containerData, boolean force) { if (force) { - LOG.info(getMessage(containerData, "Container force deleted")); + LOG.info(getMessage(containerData, "Container force deleted, CommandType=DeleteContainer")); } else { - LOG.info(getMessage(containerData, "Empty container deleted")); + LOG.info(getMessage(containerData, "Empty container deleted, CommandType=DeleteContainer")); } } @@ -128,7 +128,7 @@ public static void logDeleted(ContainerData containerData, boolean force) { * @param containerData The container that was imported to this datanode. */ public static void logImported(ContainerData containerData) { - LOG.info(getMessage(containerData)); + LOG.info(getMessage(containerData,"action=Imported")); } /** @@ -137,7 +137,7 @@ public static void logImported(ContainerData containerData) { * @param containerData The container that was exported from this datanode. */ public static void logExported(ContainerData containerData) { - LOG.info(getMessage(containerData)); + LOG.info(getMessage(containerData,"action=Exported")); } /** @@ -146,7 +146,7 @@ public static void logExported(ContainerData containerData) { * @param containerData The container that was recovered on this datanode. */ public static void logRecovered(ContainerData containerData) { - LOG.info(getMessage(containerData)); + LOG.info(getMessage(containerData,"action=Recovered")); } private static String getMessage(ContainerData containerData, @@ -159,6 +159,7 @@ private static String getMessage(ContainerData containerData) { "ID=" + containerData.getContainerID(), "Index=" + containerData.getReplicaIndex(), "BCSID=" + containerData.getBlockCommitSequenceId(), - "State=" + containerData.getState()); + "State=" + containerData.getState(), + "Volume=" + containerData.getVolume()); } } From 3fc5e577fcf41dcbad5620d926f332aa084fc79f Mon Sep 17 00:00:00 2001 From: Sreeja Chintalapati Date: Wed, 12 Feb 2025 10:16:27 +0530 Subject: [PATCH 2/4] fixed checkstyle issue --- .../ozone/container/common/utils/ContainerLogger.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/utils/ContainerLogger.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/utils/ContainerLogger.java index 2447f38808dc..f01c982f3e14 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/utils/ContainerLogger.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/utils/ContainerLogger.java @@ -51,7 +51,7 @@ private ContainerLogger() { } * @param containerData The container that was created and opened. */ public static void logOpen(ContainerData containerData) { - LOG.info(getMessage(containerData,"CommandType=CreateContainer")); + LOG.info(getMessage(containerData, "CommandType=CreateContainer")); } /** @@ -80,7 +80,7 @@ public static void logQuasiClosed(ContainerData containerData, * @param containerData The container that was closed. */ public static void logClosed(ContainerData containerData) { - LOG.info(getMessage(containerData,"CommandType=CloseContainer")); + LOG.info(getMessage(containerData, "CommandType=CloseContainer")); } /** @@ -128,7 +128,7 @@ public static void logDeleted(ContainerData containerData, boolean force) { * @param containerData The container that was imported to this datanode. */ public static void logImported(ContainerData containerData) { - LOG.info(getMessage(containerData,"action=Imported")); + LOG.info(getMessage(containerData, "action=Imported")); } /** @@ -137,7 +137,7 @@ public static void logImported(ContainerData containerData) { * @param containerData The container that was exported from this datanode. */ public static void logExported(ContainerData containerData) { - LOG.info(getMessage(containerData,"action=Exported")); + LOG.info(getMessage(containerData, "action=Exported")); } /** @@ -146,7 +146,7 @@ public static void logExported(ContainerData containerData) { * @param containerData The container that was recovered on this datanode. */ public static void logRecovered(ContainerData containerData) { - LOG.info(getMessage(containerData,"action=Recovered")); + LOG.info(getMessage(containerData, "action=Recovered")); } private static String getMessage(ContainerData containerData, From df556f60f3f8480777edbdc0c481f31ebf23e2d9 Mon Sep 17 00:00:00 2001 From: Sreeja Chintalapati Date: Tue, 6 May 2025 17:32:34 +0530 Subject: [PATCH 3/4] Added log message assertion for marking container unhealthy --- .../keyvalue/TestKeyValueHandlerWithUnhealthyContainer.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueHandlerWithUnhealthyContainer.java b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueHandlerWithUnhealthyContainer.java index 395e943c7684..3e5c619aceb0 100644 --- a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueHandlerWithUnhealthyContainer.java +++ b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueHandlerWithUnhealthyContainer.java @@ -31,8 +31,10 @@ import org.apache.hadoop.ozone.container.common.report.IncrementalReportSender; import org.apache.hadoop.ozone.container.common.statemachine.DatanodeStateMachine; +import org.apache.hadoop.ozone.container.common.utils.ContainerLogger; import org.apache.hadoop.ozone.container.common.volume.HddsVolume; import org.apache.hadoop.ozone.container.common.volume.MutableVolumeSet; +import org.apache.ozone.test.GenericTestUtils; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; @@ -58,6 +60,7 @@ import static org.apache.hadoop.ozone.container.ContainerTestHelper.getPutBlockRequest; import static org.apache.hadoop.ozone.container.ContainerTestHelper.getTestBlockID; import static org.apache.hadoop.ozone.container.ContainerTestHelper.getWriteChunkRequest; +import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.mockito.Mockito.any; import static org.mockito.Mockito.atMostOnce; @@ -78,6 +81,8 @@ public class TestKeyValueHandlerWithUnhealthyContainer { private File tempDir; private IncrementalReportSender mockIcrSender; + private final GenericTestUtils.LogCapturer logCapturer = + GenericTestUtils.LogCapturer.log4j2(ContainerLogger.LOG_NAME); @BeforeEach public void init() { @@ -240,6 +245,7 @@ public void testMarkContainerUnhealthyInFailedVolume() throws IOException { handler.markContainerUnhealthy(container, ContainerTestUtils.getUnhealthyScanResult()); verify(mockIcrSender, atMostOnce()).send(any()); + assertThat(logCapturer.getOutput()).contains("CORRUPT_CHUNK"); } // -- Helper methods below. From f076ee4a0c9990abdd0863cdff68b6b96b74d721 Mon Sep 17 00:00:00 2001 From: Sreeja Chintalapati Date: Thu, 8 May 2025 10:07:56 +0530 Subject: [PATCH 4/4] Removed unnecessary log messages and assert --- .../ozone/container/common/utils/ContainerLogger.java | 10 +++++----- .../TestKeyValueHandlerWithUnhealthyContainer.java | 6 ------ 2 files changed, 5 insertions(+), 11 deletions(-) diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/utils/ContainerLogger.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/utils/ContainerLogger.java index 3ec97f6c210d..1749d94ae915 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/utils/ContainerLogger.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/utils/ContainerLogger.java @@ -51,7 +51,7 @@ private ContainerLogger() { } * @param containerData The container that was created and opened. */ public static void logOpen(ContainerData containerData) { - LOG.info(getMessage(containerData, "CommandType=CreateContainer")); + LOG.info(getMessage(containerData)); } /** @@ -80,7 +80,7 @@ public static void logQuasiClosed(ContainerData containerData, * @param containerData The container that was closed. */ public static void logClosed(ContainerData containerData) { - LOG.info(getMessage(containerData, "CommandType=CloseContainer")); + LOG.info(getMessage(containerData)); } /** @@ -116,9 +116,9 @@ public static void logLost(ContainerData containerData, String message) { */ public static void logDeleted(ContainerData containerData, boolean force) { if (force) { - LOG.info(getMessage(containerData, "Container force deleted, CommandType=DeleteContainer")); + LOG.info(getMessage(containerData, "Container force deleted")); } else { - LOG.info(getMessage(containerData, "Empty container deleted, CommandType=DeleteContainer")); + LOG.info(getMessage(containerData, "Empty container deleted")); } } @@ -146,7 +146,7 @@ public static void logExported(ContainerData containerData) { * @param containerData The container that was recovered on this datanode. */ public static void logRecovered(ContainerData containerData) { - LOG.info(getMessage(containerData, "action=Recovered")); + LOG.info(getMessage(containerData)); } private static String getMessage(ContainerData containerData, diff --git a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueHandlerWithUnhealthyContainer.java b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueHandlerWithUnhealthyContainer.java index ba719f507b41..34a1d68a1f6a 100644 --- a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueHandlerWithUnhealthyContainer.java +++ b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueHandlerWithUnhealthyContainer.java @@ -52,17 +52,14 @@ import org.apache.hadoop.ozone.container.common.interfaces.Container; import org.apache.hadoop.ozone.container.common.report.IncrementalReportSender; import org.apache.hadoop.ozone.container.common.statemachine.DatanodeStateMachine; -import org.apache.hadoop.ozone.container.common.utils.ContainerLogger; import org.apache.hadoop.ozone.container.common.volume.HddsVolume; import org.apache.hadoop.ozone.container.common.volume.MutableVolumeSet; -import org.apache.ozone.test.GenericTestUtils; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; -import static org.assertj.core.api.Assertions.assertThat; /** * Test that KeyValueHandler fails certain operations when the @@ -74,8 +71,6 @@ public class TestKeyValueHandlerWithUnhealthyContainer { private File tempDir; private IncrementalReportSender mockIcrSender; - private final GenericTestUtils.LogCapturer logCapturer = - GenericTestUtils.LogCapturer.log4j2(ContainerLogger.LOG_NAME); @BeforeEach public void init() { @@ -237,7 +232,6 @@ public void testMarkContainerUnhealthyInFailedVolume() throws IOException { handler.markContainerUnhealthy(container, ContainerTestUtils.getUnhealthyScanResult()); verify(mockIcrSender, atMostOnce()).send(any()); - assertThat(logCapturer.getOutput()).contains("CORRUPT_CHUNK"); } // -- Helper methods below.