From 702e5e71411446b5752417446a6c85ae13ede1ce Mon Sep 17 00:00:00 2001 From: Tamas Domok Date: Fri, 20 Oct 2023 12:05:54 +0200 Subject: [PATCH] YARN-11578. Cache fs supports chmod in LogAggregationFileController. (#6120) (#6143) --- .../LogAggregationFileController.java | 73 +++++++++++++---- .../TestLogAggregationFileController.java | 82 +++++++++++++++---- 2 files changed, 122 insertions(+), 33 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 697ecbc831aad..c0655d80765ad 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 @@ -32,7 +32,9 @@ import java.util.HashSet; 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; @@ -113,6 +115,35 @@ public abstract class LogAggregationFileController { protected boolean fsSupportsChmod = true; + private static class FsLogPathKey { + private Class fsType; + private Path logPath; + + FsLogPathKey(Class 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 FS_CHMOD_CACHE + = new ConcurrentHashMap<>(); + public LogAggregationFileController() {} /** @@ -370,26 +401,34 @@ public void verifyAndCreateRemoteLogDir() { + 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 35668842247f9..4e9e02232b792 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,22 +18,29 @@ package org.apache.hadoop.yarn.logaggregation.filecontroller; +import java.io.IOException; +import java.net.URI; +import java.net.URISyntaxException; + +import org.mockito.Mockito; + import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.FileStatus; import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.Path; import org.apache.hadoop.fs.permission.FsPermission; import org.apache.hadoop.security.UserGroupInformation; +import org.apache.hadoop.yarn.conf.YarnConfiguration; import org.junit.Assert; import org.junit.Test; -import org.mockito.Mockito; -import java.net.URI; import static org.apache.hadoop.yarn.logaggregation.filecontroller.LogAggregationFileController.TLDIR_PERMISSIONS; import static org.mockito.Mockito.doNothing; +import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.any; import static org.mockito.Mockito.eq; @@ -46,30 +53,73 @@ 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(new UnsupportedOperationException()).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(new UnsupportedOperationException()).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)); + doNothing().when(controller).initInternal(any(Configuration.class)); UserGroupInformation ugi = UserGroupInformation.createUserForTesting( "yarn_user", new String[]{"yarn_group", "other_group"}); UserGroupInformation.setLoginUser(ugi); + } - doNothing().when(controller).initInternal(any(Configuration.class)); - controller.initialize(conf, "TFile"); - controller.verifyAndCreateRemoteLogDir(); - - verify(fs).createNewFile(any()); - verify(fs).setPermission(any(), eq(new FsPermission(TLDIR_PERMISSIONS))); - verify(fs).delete(any(), eq(false)); - Assert.assertTrue(controller.fsSupportsChmod); + private static void assertPermissionFileWasUsedOneTime(FileSystem fs) throws IOException { + verify(fs, times(1)).createNewFile(any()); + verify(fs, times(1)).setPermission(any(), eq(new FsPermission(TLDIR_PERMISSIONS))); + verify(fs, times(1)).delete(any(), eq(false)); } }