From fc5d1c8f8e1b43447d9635efa765692918518415 Mon Sep 17 00:00:00 2001 From: Gargi Jaiswal Date: Tue, 19 May 2026 11:51:25 +0530 Subject: [PATCH 1/3] fixed diskbalancer container logger message --- .../common/utils/ContainerLogger.java | 32 +++++++++---------- .../diskbalancer/DiskBalancerService.java | 9 ++++-- 2 files changed, 21 insertions(+), 20 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 a4eb1765b867..03b2a8a1ac8c 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 @@ -181,40 +181,38 @@ public static void logReconciled(ContainerData containerData, long oldDataChecks /** * Logged when a container is successfully moved from one data volume to another. * - * @param containerId The ID of the moved container. + * @param containerData The container after it has been moved to the destination volume. * @param sourceVolume The source volume path. * @param destinationVolume The destination volume path. * @param containerSize The size of data moved from container in bytes. * @param timeTaken The time taken for the move in milliseconds. */ - public static void logMoveSuccess(long containerId, StorageVolume sourceVolume, + public static void logMoveSuccess(ContainerData containerData, StorageVolume sourceVolume, StorageVolume destinationVolume, long containerSize, long timeTaken) { - LOG.info(getMessage(containerId, sourceVolume, destinationVolume, containerSize, timeTaken)); + LOG.info(getMessage(containerData, + "SrcVolume=" + sourceVolume, + "DestVolume=" + destinationVolume, + "Size=" + containerSize + " bytes", + "TimeTaken=" + timeTaken + " ms", + "Container is moved from SrcVolume to DestVolume")); } - private static String getMessage(ContainerData containerData, - String message) { + private static String getMessage(ContainerData containerData, String message) { return String.join(FIELD_SEPARATOR, getMessage(containerData), message); } - private static String getMessage(ContainerData containerData) { + private static String getMessage(ContainerData containerData, String... fields) { return String.join(FIELD_SEPARATOR, "ID=" + containerData.getContainerID(), "Index=" + containerData.getReplicaIndex(), "BCSID=" + containerData.getBlockCommitSequenceId(), "State=" + containerData.getState(), - "Volume=" + containerData.getVolume(), - "DataChecksum=" + checksumToString(containerData.getDataChecksum())); + String.join(FIELD_SEPARATOR, fields)); } - private static String getMessage(long containerId, StorageVolume sourceVolume, - StorageVolume destinationVolume, long containerSize, long timeTaken) { - return String.join(FIELD_SEPARATOR, - "ID=" + containerId, - "SrcVolume=" + sourceVolume, - "DestVolume=" + destinationVolume, - "Size=" + containerSize + " bytes", - "TimeTaken=" + timeTaken + " ms", - "Container is moved from SrcVolume to DestVolume"); + private static String getMessage(ContainerData containerData) { + return getMessage(containerData, + "Volume=" + containerData.getVolume(), + "DataChecksum=" + checksumToString(containerData.getDataChecksum())); } } diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/diskbalancer/DiskBalancerService.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/diskbalancer/DiskBalancerService.java index a962c0e80ff3..0a13aea91ddf 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/diskbalancer/DiskBalancerService.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/diskbalancer/DiskBalancerService.java @@ -509,6 +509,7 @@ protected class DiskBalancerTask implements BackgroundTask { public BackgroundTaskResult call() { long startTime = Time.monotonicNow(); boolean moveSucceeded = true; + Container newContainer = null; long containerId = containerData.getContainerID(); Container container = ozoneContainer.getContainerSet().getContainer(containerId); boolean readLockReleased = false; @@ -580,7 +581,7 @@ public BackgroundTaskResult call() { } // Import the container. importContainer will reset container back to original state - Container newContainer = ozoneContainer.getController().importContainer(tempContainerData); + newContainer = ozoneContainer.getController().importContainer(tempContainerData); // Step 4: Update container for containerID and mark old container for deletion // first, update the in-memory set to point to the new replica. @@ -636,8 +637,10 @@ public BackgroundTaskResult call() { if (moveSucceeded) { // Add current old container to pendingDeletionContainers. pendingDeletionContainers.put(System.currentTimeMillis() + replicaDeletionDelay, container); - ContainerLogger.logMoveSuccess(containerId, sourceVolume, - destVolume, containerSize, Time.monotonicNow() - startTime); + if (newContainer != null) { + ContainerLogger.logMoveSuccess(newContainer.getContainerData(), sourceVolume, + destVolume, containerSize, Time.monotonicNow() - startTime); + } } postCall(moveSucceeded, startTime); From 9ed51fcd5f65595aac6ddae6b6a2466e94eb8e06 Mon Sep 17 00:00:00 2001 From: Gargi Jaiswal Date: Wed, 20 May 2026 12:46:40 +0530 Subject: [PATCH 2/3] address copilot comment --- .../container/diskbalancer/DiskBalancerService.java | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/diskbalancer/DiskBalancerService.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/diskbalancer/DiskBalancerService.java index 0a13aea91ddf..a75acac53b47 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/diskbalancer/DiskBalancerService.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/diskbalancer/DiskBalancerService.java @@ -634,15 +634,14 @@ public BackgroundTaskResult call() { if (!readLockReleased) { container.readUnlock(); } - if (moveSucceeded) { + boolean moveCompleted = moveSucceeded && newContainer != null; + if (moveCompleted) { // Add current old container to pendingDeletionContainers. pendingDeletionContainers.put(System.currentTimeMillis() + replicaDeletionDelay, container); - if (newContainer != null) { - ContainerLogger.logMoveSuccess(newContainer.getContainerData(), sourceVolume, - destVolume, containerSize, Time.monotonicNow() - startTime); - } + ContainerLogger.logMoveSuccess(newContainer.getContainerData(), sourceVolume, + destVolume, containerSize, Time.monotonicNow() - startTime); } - postCall(moveSucceeded, startTime); + postCall(moveCompleted, startTime); // pick one expired container from pendingDeletionContainers to delete tryCleanupOnePendingDeletionContainer(); From 9ae17550c2cc0019828900ed92770aa9733c3b91 Mon Sep 17 00:00:00 2001 From: Gargi Jaiswal Date: Wed, 20 May 2026 13:13:13 +0530 Subject: [PATCH 3/3] fix findbugs --- .../ozone/container/diskbalancer/DiskBalancerService.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/diskbalancer/DiskBalancerService.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/diskbalancer/DiskBalancerService.java index a75acac53b47..1d065024dfb1 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/diskbalancer/DiskBalancerService.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/diskbalancer/DiskBalancerService.java @@ -634,14 +634,13 @@ public BackgroundTaskResult call() { if (!readLockReleased) { container.readUnlock(); } - boolean moveCompleted = moveSucceeded && newContainer != null; - if (moveCompleted) { + if (moveSucceeded && newContainer != null) { // Add current old container to pendingDeletionContainers. pendingDeletionContainers.put(System.currentTimeMillis() + replicaDeletionDelay, container); ContainerLogger.logMoveSuccess(newContainer.getContainerData(), sourceVolume, destVolume, containerSize, Time.monotonicNow() - startTime); } - postCall(moveCompleted, startTime); + postCall(moveSucceeded && newContainer != null, startTime); // pick one expired container from pendingDeletionContainers to delete tryCleanupOnePendingDeletionContainer();