You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-commits@hadoop.apache.org by bt...@apache.org on 2023/10/20 16:47:19 UTC
[hadoop] branch branch-3.3 updated: YARN-11578. Cache fs supports chmod in LogAggregationFileController. (#6120) (#6142)
This is an automated email from the ASF dual-hosted git repository.
bteke pushed a commit to branch branch-3.3
in repository https://gitbox.apache.org/repos/asf/hadoop.git
The following commit(s) were added to refs/heads/branch-3.3 by this push:
new c83104140492 YARN-11578. Cache fs supports chmod in LogAggregationFileController. (#6120) (#6142)
c83104140492 is described below
commit c8310414049203d26cc2b4709a2b299eec9009d9
Author: Tamas Domok <td...@cloudera.com>
AuthorDate: Fri Oct 20 18:47:10 2023 +0200
YARN-11578. Cache fs supports chmod in LogAggregationFileController. (#6120) (#6142)
---
.../LogAggregationFileController.java | 73 +++++++++++++-----
.../TestLogAggregationFileController.java | 87 +++++++++++++++++-----
2 files changed, 125 insertions(+), 35 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 4844ae2b15fe..84057118fe86 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;
@@ -108,6 +110,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() {}
/**
@@ -375,26 +406,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 suppports 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 818e01129fc6..2b3fc8eb2d6e 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
@@ -18,6 +18,14 @@
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.mockito.ArgumentMatcher;
+import org.mockito.Mockito;
+
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.fs.FileStatus;
import org.apache.hadoop.fs.FileSystem;
@@ -27,11 +35,7 @@ import org.apache.hadoop.security.UserGroupInformation;
import org.apache.hadoop.yarn.conf.YarnConfiguration;
import org.junit.Assert;
import org.junit.Test;
-import org.mockito.ArgumentMatcher;
-import org.mockito.Mockito;
-import java.io.FileNotFoundException;
-import java.net.URI;
import static org.apache.hadoop.yarn.logaggregation.filecontroller.LogAggregationFileController.TLDIR_PERMISSIONS;
import static org.mockito.ArgumentMatchers.any;
@@ -40,6 +44,7 @@ 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;
/**
@@ -115,30 +120,76 @@ public class TestLogAggregationFileController {
@Test
public 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);
+ Assert.assertTrue(controller.fsSupportsChmod);
+
+ doThrow(UnsupportedOperationException.class).when(fs).setPermission(any(), any());
+ controller.verifyAndCreateRemoteLogDir();
+ assertPermissionFileWasUsedOneTime(fs); // still once -> cached
+ Assert.assertTrue(controller.fsSupportsChmod);
+
+ controller.fsSupportsChmod = false;
+ controller.verifyAndCreateRemoteLogDir();
+ assertPermissionFileWasUsedOneTime(fs); // still once -> cached
+ Assert.assertTrue(controller.fsSupportsChmod);
+ }
+
+ @Test
+ public 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);
+ Assert.assertFalse(controller.fsSupportsChmod);
+
+ controller.verifyAndCreateRemoteLogDir();
+ assertPermissionFileWasUsedOneTime(fs); // still once -> cached
+ Assert.assertFalse(controller.fsSupportsChmod);
+
controller.fsSupportsChmod = true;
+ controller.verifyAndCreateRemoteLogDir();
+ assertPermissionFileWasUsedOneTime(fs); // still once -> cached
+ Assert.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")),
- eq(new FsPermission(TLDIR_PERMISSIONS)));
- verify(fs).delete(argThat(new PathContainsString(".permission_check")), eq(false));
- Assert.assertTrue(controller.fsSupportsChmod);
+ 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, times(1))
+ .delete(argThat(new PathContainsString(".permission_check")), eq(false));
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: common-commits-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-commits-help@hadoop.apache.org