This is an automated email from the ASF dual-hosted git repository.
bteke pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/hadoop.git
The following commit(s) were added to refs/heads/trunk by this push:
new a04a9e107b57 YARN-11578. Cache fs supports chmod in
LogAggregationFileController. (#6120)
a04a9e107b57 is described below
commit a04a9e107b57afb37740ac73ca239d3c335f8695
Author: Tamas Domok <[email protected]>
AuthorDate: Mon Oct 2 15:20:47 2023 +0200
YARN-11578. Cache fs supports chmod in LogAggregationFileController. (#6120)
---
.../LogAggregationFileController.java | 73 ++++++++++++++++-----
.../TestLogAggregationFileController.java | 76 ++++++++++++++++++----
2 files changed, 119 insertions(+), 30 deletions(-)
diff --git
a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/logaggregation/filecontroller/LogAggregationFileController.java
b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/logaggregation/filecontroller/LogAggregationFileController.java
index 7803c6215f20..8fe4f828ebf2 100644
---
a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/logaggregation/filecontroller/LogAggregationFileController.java
+++
b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/logaggregation/filecontroller/LogAggregationFileController.java
@@ -33,7 +33,9 @@ import java.util.HashSet;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
+import java.util.Objects;
import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
import java.util.stream.Collectors;
import org.apache.commons.lang3.StringUtils;
import org.apache.hadoop.classification.InterfaceAudience.Public;
@@ -111,6 +113,35 @@ public abstract class LogAggregationFileController {
protected boolean fsSupportsChmod = true;
+ private static class FsLogPathKey {
+ private Class<? extends FileSystem> fsType;
+ private Path logPath;
+
+ FsLogPathKey(Class<? extends FileSystem> fsType, Path logPath) {
+ this.fsType = fsType;
+ this.logPath = logPath;
+ }
+
+ @Override
+ public boolean equals(Object o) {
+ if (this == o) {
+ return true;
+ }
+ if (o == null || getClass() != o.getClass()) {
+ return false;
+ }
+ FsLogPathKey that = (FsLogPathKey) o;
+ return Objects.equals(fsType, that.fsType) && Objects.equals(logPath,
that.logPath);
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(fsType, logPath);
+ }
+ }
+ private static final ConcurrentHashMap<FsLogPathKey, Boolean> FS_CHMOD_CACHE
+ = new ConcurrentHashMap<>();
+
public LogAggregationFileController() {}
/**
@@ -429,26 +460,34 @@ public abstract class LogAggregationFileController {
+ remoteRootLogDir + "]", e);
}
} else {
- //Check if FS has capability to set/modify permissions
- Path permissionCheckFile = new Path(qualified,
String.format("%s.permission_check",
- RandomStringUtils.randomAlphanumeric(8)));
+ final FsLogPathKey key = new FsLogPathKey(remoteFS.getClass(),
qualified);
+ FileSystem finalRemoteFS = remoteFS;
+ fsSupportsChmod = FS_CHMOD_CACHE.computeIfAbsent(key,
+ k -> checkFsSupportsChmod(finalRemoteFS, remoteRootLogDir,
qualified));
+ }
+ }
+
+ private boolean checkFsSupportsChmod(FileSystem remoteFS, Path logDir, Path
qualified) {
+ //Check if FS has capability to set/modify permissions
+ Path permissionCheckFile = new Path(qualified,
String.format("%s.permission_check",
+ RandomStringUtils.randomAlphanumeric(8)));
+ try {
+ remoteFS.createNewFile(permissionCheckFile);
+ remoteFS.setPermission(permissionCheckFile, new
FsPermission(TLDIR_PERMISSIONS));
+ return true;
+ } catch (UnsupportedOperationException use) {
+ LOG.info("Unable to set permissions for configured filesystem since"
+ + " it does not support this {}", remoteFS.getScheme());
+ } catch (IOException e) {
+ LOG.warn("Failed to check if FileSystem supports permissions on "
+ + "remoteLogDir [{}]", logDir, e);
+ } finally {
try {
- remoteFS.createNewFile(permissionCheckFile);
- remoteFS.setPermission(permissionCheckFile, new
FsPermission(TLDIR_PERMISSIONS));
- } catch (UnsupportedOperationException use) {
- LOG.info("Unable to set permissions for configured filesystem since"
- + " it does not support this {}", remoteFS.getScheme());
- fsSupportsChmod = false;
- } catch (IOException e) {
- LOG.warn("Failed to check if FileSystem supports permissions on "
- + "remoteLogDir [" + remoteRootLogDir + "]", e);
- } finally {
- try {
- remoteFS.delete(permissionCheckFile, false);
- } catch (IOException ignored) {
- }
+ remoteFS.delete(permissionCheckFile, false);
+ } catch (IOException ignored) {
}
}
+ return false;
}
/**
diff --git
a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/logaggregation/filecontroller/TestLogAggregationFileController.java
b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/logaggregation/filecontroller/TestLogAggregationFileController.java
index fe1c5f2fa732..a7c653b8187c 100644
---
a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/logaggregation/filecontroller/TestLogAggregationFileController.java
+++
b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/logaggregation/filecontroller/TestLogAggregationFileController.java
@@ -19,7 +19,9 @@
package org.apache.hadoop.yarn.logaggregation.filecontroller;
import java.io.FileNotFoundException;
+import java.io.IOException;
import java.net.URI;
+import java.net.URISyntaxException;
import org.junit.jupiter.api.Test;
import org.mockito.ArgumentMatcher;
@@ -35,12 +37,14 @@ import org.apache.hadoop.yarn.conf.YarnConfiguration;
import static
org.apache.hadoop.yarn.logaggregation.filecontroller.LogAggregationFileController.TLDIR_PERMISSIONS;
import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.argThat;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
/**
@@ -116,30 +120,76 @@ public class TestLogAggregationFileController {
@Test
void testRemoteDirCreationWithCustomUser() throws Exception {
+ LogAggregationFileController controller = mock(
+ LogAggregationFileController.class, Mockito.CALLS_REAL_METHODS);
FileSystem fs = mock(FileSystem.class);
- doReturn(new URI("")).when(fs).getUri();
- doReturn(new FileStatus(128, false, 0, 64, System.currentTimeMillis(),
- System.currentTimeMillis(), new FsPermission(TLDIR_PERMISSIONS),
- "not_yarn_user", "yarn_group", new Path("/tmp/logs"))).when(fs)
- .getFileStatus(any(Path.class));
+ setupCustomUserMocks(controller, fs, "/tmp/logs");
- Configuration conf = new Configuration();
+ controller.initialize(new Configuration(), "TFile");
+ controller.fsSupportsChmod = false;
+
+ controller.verifyAndCreateRemoteLogDir();
+ assertPermissionFileWasUsedOneTime(fs);
+ assertTrue(controller.fsSupportsChmod);
+
+ doThrow(UnsupportedOperationException.class).when(fs).setPermission(any(),
any());
+ controller.verifyAndCreateRemoteLogDir();
+ assertPermissionFileWasUsedOneTime(fs); // still once -> cached
+ assertTrue(controller.fsSupportsChmod);
+
+ controller.fsSupportsChmod = false;
+ controller.verifyAndCreateRemoteLogDir();
+ assertPermissionFileWasUsedOneTime(fs); // still once -> cached
+ assertTrue(controller.fsSupportsChmod);
+ }
+
+ @Test
+ void testRemoteDirCreationWithCustomUserFsChmodNotSupported() throws
Exception {
LogAggregationFileController controller = mock(
LogAggregationFileController.class, Mockito.CALLS_REAL_METHODS);
+ FileSystem fs = mock(FileSystem.class);
+ setupCustomUserMocks(controller, fs, "/tmp/logs2");
+ doThrow(UnsupportedOperationException.class).when(fs).setPermission(any(),
any());
+
+ Configuration conf = new Configuration();
+ conf.set(YarnConfiguration.NM_REMOTE_APP_LOG_DIR, "/tmp/logs2");
+ controller.initialize(conf, "TFile");
+ controller.verifyAndCreateRemoteLogDir();
+ assertPermissionFileWasUsedOneTime(fs);
+ assertFalse(controller.fsSupportsChmod);
+
+ controller.verifyAndCreateRemoteLogDir();
+ assertPermissionFileWasUsedOneTime(fs); // still once -> cached
+ assertFalse(controller.fsSupportsChmod);
+
controller.fsSupportsChmod = true;
+ controller.verifyAndCreateRemoteLogDir();
+ assertPermissionFileWasUsedOneTime(fs); // still once -> cached
+ assertFalse(controller.fsSupportsChmod);
+ }
+
+ private static void setupCustomUserMocks(LogAggregationFileController
controller,
+ FileSystem fs, String path)
+ throws URISyntaxException, IOException {
+ doReturn(new URI("")).when(fs).getUri();
+ doReturn(new FileStatus(128, false, 0, 64, System.currentTimeMillis(),
+ System.currentTimeMillis(), new FsPermission(TLDIR_PERMISSIONS),
+ "not_yarn_user", "yarn_group", new Path(path))).when(fs)
+ .getFileStatus(any(Path.class));
doReturn(fs).when(controller).getFileSystem(any(Configuration.class));
UserGroupInformation ugi = UserGroupInformation.createUserForTesting(
"yarn_user", new String[]{"yarn_group", "other_group"});
UserGroupInformation.setLoginUser(ugi);
+ }
- controller.initialize(conf, "TFile");
- controller.verifyAndCreateRemoteLogDir();
-
- verify(fs).createNewFile(argThat(new
PathContainsString(".permission_check")));
- verify(fs).setPermission(argThat(new
PathContainsString(".permission_check")),
+ private static void assertPermissionFileWasUsedOneTime(FileSystem fs) throws
IOException {
+ verify(fs, times(1))
+ .createNewFile(argThat(new PathContainsString(".permission_check")));
+ verify(fs, times(1))
+ .setPermission(argThat(new PathContainsString(".permission_check")),
eq(new FsPermission(TLDIR_PERMISSIONS)));
- verify(fs).delete(argThat(new PathContainsString(".permission_check")),
eq(false));
- assertTrue(controller.fsSupportsChmod);
+ verify(fs, times(1))
+ .delete(argThat(new PathContainsString(".permission_check")),
eq(false));
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
