This is an automated email from the ASF dual-hosted git repository.
xianjin 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 7dcaeeb2 [#571] feat: skip init for empty writable base dir (#573)
7dcaeeb2 is described below
commit 7dcaeeb247520bd326e51e70a922db967419a229
Author: advancedxy <[email protected]>
AuthorDate: Thu Feb 9 16:57:36 2023 +0800
[#571] feat: skip init for empty writable base dir (#573)
### What changes were proposed in this pull request?
Skip init empty and writable base dir
### Why are the changes needed?
In some deployment env, base dir could be a mount point
This fixes #571
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
Added UT
---
.../uniffle/storage/common/LocalStorage.java | 21 ++++++++++++++++---
.../uniffle/storage/common/LocalStorageTest.java | 24 ++++++++++++++++++++++
2 files changed, 42 insertions(+), 3 deletions(-)
diff --git
a/storage/src/main/java/org/apache/uniffle/storage/common/LocalStorage.java
b/storage/src/main/java/org/apache/uniffle/storage/common/LocalStorage.java
index e1f9b1cd..000a9aa5 100644
--- a/storage/src/main/java/org/apache/uniffle/storage/common/LocalStorage.java
+++ b/storage/src/main/java/org/apache/uniffle/storage/common/LocalStorage.java
@@ -19,8 +19,10 @@ package org.apache.uniffle.storage.common;
import java.io.File;
import java.io.IOException;
+import java.nio.file.DirectoryStream;
import java.nio.file.FileStore;
import java.nio.file.Files;
+import java.nio.file.Path;
import java.util.HashSet;
import java.util.List;
import java.util.Queue;
@@ -76,9 +78,13 @@ public class LocalStorage extends AbstractStorage {
File baseFolder = new File(basePath);
try {
- FileUtils.deleteDirectory(baseFolder);
- if (!baseFolder.mkdirs()) {
- throw new IOException("Failed to create base folder: " + basePath);
+ if (isEmptyAndWritableDir(baseFolder)) {
+ LOG.warn("Base directory is already an empty dir, skip init");
+ } else {
+ FileUtils.deleteDirectory(baseFolder);
+ if (!baseFolder.mkdirs()) {
+ throw new IOException("Failed to create base folder: " + basePath);
+ }
}
FileStore store = Files.getFileStore(baseFolder.toPath());
this.mountPoint = store.name();
@@ -99,6 +105,15 @@ public class LocalStorage extends AbstractStorage {
}
}
+ private boolean isEmptyAndWritableDir(File baseDir) throws IOException {
+ if (baseDir.isDirectory() && baseDir.canWrite()) {
+ try (DirectoryStream<Path> stream =
Files.newDirectoryStream(baseDir.toPath())) {
+ return !stream.iterator().hasNext();
+ }
+ }
+ return false;
+ }
+
@Override
public String getStoragePath() {
return basePath;
diff --git
a/storage/src/test/java/org/apache/uniffle/storage/common/LocalStorageTest.java
b/storage/src/test/java/org/apache/uniffle/storage/common/LocalStorageTest.java
index 49082c6c..1ed83de8 100644
---
a/storage/src/test/java/org/apache/uniffle/storage/common/LocalStorageTest.java
+++
b/storage/src/test/java/org/apache/uniffle/storage/common/LocalStorageTest.java
@@ -20,9 +20,12 @@ package org.apache.uniffle.storage.common;
import java.io.File;
import java.io.IOException;
import java.nio.file.Files;
+import java.nio.file.attribute.PosixFilePermission;
import java.util.List;
+import java.util.Set;
import com.google.common.collect.Lists;
+import com.google.common.collect.Sets;
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
@@ -43,12 +46,15 @@ import static org.junit.jupiter.api.Assertions.assertTrue;
public class LocalStorageTest {
private static File testBaseDir;
+ private static File testBaseDirWithoutPermission;
private static String mountPoint;
@BeforeAll
public static void setUp(@TempDir File tempDir) throws IOException {
testBaseDir = new File(tempDir, "test");
+ testBaseDirWithoutPermission = new File(tempDir, "test-no-permission");
testBaseDir.mkdir();
+ testBaseDirWithoutPermission.mkdirs();
try {
mountPoint = Files.getFileStore(testBaseDir.toPath()).name();
} catch (IOException ioe) {
@@ -83,6 +89,24 @@ public class LocalStorageTest {
assertTrue(item.canWrite());
}
+ @Test
+ public void unRemovableEmptyDirShouldNotFail() throws IOException {
+ File newBaseDir = new File(testBaseDirWithoutPermission, "test-new");
+ assertTrue(newBaseDir.mkdirs());
+ // then remove write permission to parent dir.
+ Set<PosixFilePermission> perms = Sets.newHashSet();
+ perms.add(PosixFilePermission.OWNER_READ);
+ perms.add(PosixFilePermission.OWNER_EXECUTE);
+ Files.setPosixFilePermissions(testBaseDirWithoutPermission.toPath(),
perms);
+ LocalStorage item =
LocalStorage.newBuilder().basePath(newBaseDir.getAbsolutePath())
+ .cleanupThreshold(50)
+ .highWaterMarkOfWrite(95)
+ .lowWaterMarkOfWrite(80)
+ .capacity(100)
+ .cleanIntervalMs(5000)
+ .build();
+ }
+
@Test
public void removeResourcesTest() throws Exception {
LocalStorage item = prepareDiskItem();
