From 31c96829c767a093d6de8bdc30e06089eeba860b Mon Sep 17 00:00:00 2001 From: Jason Darrell Lowe Date: Wed, 29 May 2013 14:25:04 +0000 Subject: [PATCH] YARN-512. Log aggregation root directory check is more expensive than it needs to be. Contributed by Maysam Yabandeh git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/trunk@1487498 13f79535-47bb-0310-9956-ffa450edef68 --- hadoop-yarn-project/CHANGES.txt | 3 ++ .../logaggregation/LogAggregationService.java | 40 +++++++++---------- .../TestLogAggregationService.java | 24 +++++++++++ 3 files changed, 45 insertions(+), 22 deletions(-) diff --git a/hadoop-yarn-project/CHANGES.txt b/hadoop-yarn-project/CHANGES.txt index 1f7365f02d..3bfcaefb23 100644 --- a/hadoop-yarn-project/CHANGES.txt +++ b/hadoop-yarn-project/CHANGES.txt @@ -223,6 +223,9 @@ Release 2.0.5-beta - UNRELEASED OPTIMIZATIONS + YARN-512. Log aggregation root directory check is more expensive than it + needs to be. (Maysam Yabandeh via jlowe) + BUG FIXES YARN-383. AMRMClientImpl should handle null rmClient in stop() diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/logaggregation/LogAggregationService.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/logaggregation/LogAggregationService.java index f183d9e8ab..9567b60933 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/logaggregation/LogAggregationService.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/logaggregation/LogAggregationService.java @@ -18,6 +18,7 @@ package org.apache.hadoop.yarn.server.nodemanager.containermanager.logaggregation; +import java.io.FileNotFoundException; import java.io.IOException; import java.security.PrivilegedExceptionAction; import java.util.Map; @@ -163,36 +164,31 @@ private void stopAggregators() { } void verifyAndCreateRemoteLogDir(Configuration conf) { - // Checking the existance of the TLD + // Checking the existence of the TLD FileSystem remoteFS = null; try { remoteFS = FileSystem.get(conf); } catch (IOException e) { throw new YarnException("Unable to get Remote FileSystem instance", e); } - boolean remoteExists = false; + boolean remoteExists = true; try { - remoteExists = remoteFS.exists(this.remoteRootLogDir); - } catch (IOException e) { - throw new YarnException("Failed to check for existence of remoteLogDir [" - + this.remoteRootLogDir + "]", e); - } - if (remoteExists) { - try { - FsPermission perms = - remoteFS.getFileStatus(this.remoteRootLogDir).getPermission(); - if (!perms.equals(TLDIR_PERMISSIONS)) { - LOG.warn("Remote Root Log Dir [" + this.remoteRootLogDir - + "] already exist, but with incorrect permissions. " - + "Expected: [" + TLDIR_PERMISSIONS + "], Found: [" + perms - + "]." + " The cluster may have problems with multiple users."); - } - } catch (IOException e) { - throw new YarnException( - "Failed to check permissions for dir [" - + this.remoteRootLogDir + "]", e); + FsPermission perms = + remoteFS.getFileStatus(this.remoteRootLogDir).getPermission(); + if (!perms.equals(TLDIR_PERMISSIONS)) { + LOG.warn("Remote Root Log Dir [" + this.remoteRootLogDir + + "] already exist, but with incorrect permissions. " + + "Expected: [" + TLDIR_PERMISSIONS + "], Found: [" + perms + + "]." + " The cluster may have problems with multiple users."); } - } else { + } catch (FileNotFoundException e) { + remoteExists = false; + } catch (IOException e) { + throw new YarnException( + "Failed to check permissions for dir [" + + this.remoteRootLogDir + "]", e); + } + if (!remoteExists) { LOG.warn("Remote Root Log Dir [" + this.remoteRootLogDir + "] does not exist. Attempting to create it."); try { diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/logaggregation/TestLogAggregationService.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/logaggregation/TestLogAggregationService.java index 8228a88fbb..43a5401ab4 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/logaggregation/TestLogAggregationService.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/logaggregation/TestLogAggregationService.java @@ -465,6 +465,30 @@ public void testVerifyAndCreateRemoteDirsFailure() logAggregationService.stop(); } + + @Test + public void testVerifyAndCreateRemoteDirNonExistence() + throws Exception { + this.conf.set(YarnConfiguration.NM_LOG_DIRS, localLogDir.getAbsolutePath()); + File aNewFile = new File(String.valueOf("tmp"+System.currentTimeMillis())); + this.conf.set(YarnConfiguration.NM_REMOTE_APP_LOG_DIR, + aNewFile.getAbsolutePath()); + + DrainDispatcher dispatcher = createDispatcher(); + LogAggregationService logAggregationService = spy( + new LogAggregationService(dispatcher, this.context, this.delSrvc, + super.dirsHandler)); + logAggregationService.init(this.conf); + boolean existsBefore = aNewFile.exists(); + assertTrue("The new file already exists!", !existsBefore); + + logAggregationService.verifyAndCreateRemoteLogDir(this.conf); + + boolean existsAfter = aNewFile.exists(); + assertTrue("The new aggregate file is not successfully created", existsAfter); + aNewFile.delete(); //housekeeping + } + @Test @SuppressWarnings("unchecked") public void testLogAggregationInitAppFailsWithoutKillingNM() throws Exception {