This is an automated email from the ASF dual-hosted git repository.
maobaolong pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-uniffle.git
The following commit(s) were added to refs/heads/master by this push:
new 99eaa9d27 [#2251] fix(server): fix wrong disk used space while config
multiply dirs in one mount point (#2252)
99eaa9d27 is described below
commit 99eaa9d27c6cc585480b4d7d300b141d77ae6f2c
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);
}
