This is an automated email from the ASF dual-hosted git repository. roryqi pushed a commit to branch branch-0.10 in repository https://gitbox.apache.org/repos/asf/incubator-uniffle.git
commit da96ff3004d55cd08d3ade85b5eb0403d0a364fc Author: maobaolong <[email protected]> AuthorDate: Mon Nov 18 20:14:06 2024 +0800 [#2251] fix(server): fix wrong disk used space while config multiply dirs in one mount point (#2252) ### What changes were proposed in this pull request? fix wrong disk used space while config multiply dirs in one mount point Fix: [#2251](https://github.com/apache/incubator-uniffle/issues/2251) ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? No need. --- .../apache/uniffle/common/storage/StorageInfo.java | 4 +++ .../java/org/apache/uniffle/server/Checker.java | 5 +++- .../server/storage/LocalStorageManager.java | 9 +++++-- .../apache/uniffle/server/HealthyMockChecker.java | 2 +- .../uniffle/server/UnHealthyMockChecker.java | 2 +- .../server/storage/LocalStorageManagerTest.java | 30 +++++++++++++++++++--- 6 files changed, 43 insertions(+), 9 deletions(-) diff --git a/common/src/main/java/org/apache/uniffle/common/storage/StorageInfo.java b/common/src/main/java/org/apache/uniffle/common/storage/StorageInfo.java index 6b31301f0..f658f3454 100644 --- a/common/src/main/java/org/apache/uniffle/common/storage/StorageInfo.java +++ b/common/src/main/java/org/apache/uniffle/common/storage/StorageInfo.java @@ -96,6 +96,10 @@ public class StorageInfo { return type; } + public long getUsedBytes() { + return usedBytes; + } + @Override public boolean equals(Object o) { if (this == o) { diff --git a/server/src/main/java/org/apache/uniffle/server/Checker.java b/server/src/main/java/org/apache/uniffle/server/Checker.java index 9c50a2f80..913d09ee3 100644 --- a/server/src/main/java/org/apache/uniffle/server/Checker.java +++ b/server/src/main/java/org/apache/uniffle/server/Checker.java @@ -17,9 +17,12 @@ package org.apache.uniffle.server; +import com.google.common.annotations.VisibleForTesting; + public abstract class Checker { Checker(ShuffleServerConf conf) {} - abstract boolean checkIsHealthy(); + @VisibleForTesting + public abstract boolean checkIsHealthy(); } diff --git a/server/src/main/java/org/apache/uniffle/server/storage/LocalStorageManager.java b/server/src/main/java/org/apache/uniffle/server/storage/LocalStorageManager.java index c33c17f3b..622b5205a 100644 --- a/server/src/main/java/org/apache/uniffle/server/storage/LocalStorageManager.java +++ b/server/src/main/java/org/apache/uniffle/server/storage/LocalStorageManager.java @@ -431,8 +431,6 @@ public class LocalStorageManager extends SingleStorageManager { Map<String, StorageInfo> result = Maps.newHashMap(); for (LocalStorage storage : localStorages) { String mountPoint = storage.getMountPoint(); - long capacity = storage.getCapacity(); - long wroteBytes = storage.getServiceUsedBytes(); StorageStatus status = StorageStatus.NORMAL; if (storage.isCorrupted()) { status = StorageStatus.UNHEALTHY; @@ -443,6 +441,13 @@ public class LocalStorageManager extends SingleStorageManager { if (media == null) { media = StorageMedia.UNKNOWN; } + StorageInfo existingMountPoint = result.get(mountPoint); + long wroteBytes = storage.getServiceUsedBytes(); + // if there is already a storage on the mount point, we should sum up the used bytes. + if (existingMountPoint != null) { + wroteBytes += existingMountPoint.getUsedBytes(); + } + long capacity = storage.getCapacity(); StorageInfo info = new StorageInfo(mountPoint, media, capacity, wroteBytes, status); result.put(mountPoint, info); } diff --git a/server/src/test/java/org/apache/uniffle/server/HealthyMockChecker.java b/server/src/test/java/org/apache/uniffle/server/HealthyMockChecker.java index 1fce9cd3b..d8950d3fb 100644 --- a/server/src/test/java/org/apache/uniffle/server/HealthyMockChecker.java +++ b/server/src/test/java/org/apache/uniffle/server/HealthyMockChecker.java @@ -25,7 +25,7 @@ public class HealthyMockChecker extends Checker { } @Override - boolean checkIsHealthy() { + public boolean checkIsHealthy() { return true; } } diff --git a/server/src/test/java/org/apache/uniffle/server/UnHealthyMockChecker.java b/server/src/test/java/org/apache/uniffle/server/UnHealthyMockChecker.java index 18e673fb4..e41649203 100644 --- a/server/src/test/java/org/apache/uniffle/server/UnHealthyMockChecker.java +++ b/server/src/test/java/org/apache/uniffle/server/UnHealthyMockChecker.java @@ -25,7 +25,7 @@ public class UnHealthyMockChecker extends Checker { } @Override - boolean checkIsHealthy() { + public boolean checkIsHealthy() { return false; } } diff --git a/server/src/test/java/org/apache/uniffle/server/storage/LocalStorageManagerTest.java b/server/src/test/java/org/apache/uniffle/server/storage/LocalStorageManagerTest.java index 374c7a14f..6802a4da2 100644 --- a/server/src/test/java/org/apache/uniffle/server/storage/LocalStorageManagerTest.java +++ b/server/src/test/java/org/apache/uniffle/server/storage/LocalStorageManagerTest.java @@ -22,6 +22,8 @@ import java.io.File; import java.io.IOException; import java.io.InputStreamReader; import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -281,20 +283,39 @@ public class LocalStorageManagerTest { } @Test - public void testGetLocalStorageInfo() { - String[] storagePaths = {"/tmp/rss-data1", "/tmp/rss-data2", "/tmp/rss-data3"}; - + public void testGetLocalStorageInfo() throws IOException { + Path testBaseDir = Files.createTempDirectory("rss-test"); + final Path storageBaseDir1 = + Files.createDirectory(Paths.get(testBaseDir.toString(), "rss-data-1")); + final Path storageBaseDir2 = + Files.createDirectory(Paths.get(testBaseDir.toString(), "rss-data-2")); + final Path storageBaseDir3 = + Files.createDirectory(Paths.get(testBaseDir.toString(), "rss-data-3")); + String[] storagePaths = { + storageBaseDir1.toString(), storageBaseDir2.toString(), storageBaseDir3.toString(), + }; ShuffleServerConf conf = new ShuffleServerConf(); conf.set(ShuffleServerConf.RSS_STORAGE_BASE_PATH, Arrays.asList(storagePaths)); conf.setLong(ShuffleServerConf.DISK_CAPACITY, 1024L); conf.setString( ShuffleServerConf.RSS_STORAGE_TYPE.key(), org.apache.uniffle.storage.util.StorageType.LOCALFILE.name()); + conf.setDouble(ShuffleServerConf.HEALTH_STORAGE_MAX_USAGE_PERCENTAGE, 100.0D); LocalStorageManager localStorageManager = new LocalStorageManager(conf); + // Create and write 3 files in each storage dir + final Path file1 = Files.createFile(Paths.get(storageBaseDir1.toString(), "partition1.data")); + final Path file2 = Files.createFile(Paths.get(storageBaseDir2.toString(), "partition2.data")); + final Path file3 = Files.createFile(Paths.get(storageBaseDir3.toString(), "partition3.data")); + FileUtils.writeByteArrayToFile(file1.toFile(), new byte[] {0x1}); + FileUtils.writeByteArrayToFile(file2.toFile(), new byte[] {0x2}); + FileUtils.writeByteArrayToFile(file3.toFile(), new byte[] {0x3}); + + boolean healthy = localStorageManager.getStorageChecker().checkIsHealthy(); + assertTrue(healthy, "should be healthy"); Map<String, StorageInfo> storageInfo = localStorageManager.getStorageInfo(); assertEquals(1, storageInfo.size()); try { - final String path = "/tmp"; + final String path = testBaseDir.toString(); final String mountPoint = Files.getFileStore(new File(path).toPath()).name(); assertNotNull(storageInfo.get(mountPoint)); // on Linux environment, it can detect SSD as local storage type @@ -315,6 +336,7 @@ public class LocalStorageManagerTest { assertEquals(StorageMedia.HDD, storageInfo.get(mountPoint).getType()); } assertEquals(StorageStatus.NORMAL, storageInfo.get(mountPoint).getStatus()); + assertEquals(3L, storageInfo.get(mountPoint).getUsedBytes()); } catch (IOException e) { throw new RuntimeException(e); }
