Return-Path: Delivered-To: apmail-hadoop-mapreduce-commits-archive@minotaur.apache.org Received: (qmail 85111 invoked from network); 25 Jan 2011 01:08:56 -0000 Received: from hermes.apache.org (HELO mail.apache.org) (140.211.11.3) by minotaur.apache.org with SMTP; 25 Jan 2011 01:08:56 -0000 Received: (qmail 1892 invoked by uid 500); 25 Jan 2011 01:08:56 -0000 Delivered-To: apmail-hadoop-mapreduce-commits-archive@hadoop.apache.org Received: (qmail 1830 invoked by uid 500); 25 Jan 2011 01:08:55 -0000 Mailing-List: contact mapreduce-commits-help@hadoop.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: mapreduce-dev@hadoop.apache.org Delivered-To: mailing list mapreduce-commits@hadoop.apache.org Received: (qmail 1822 invoked by uid 99); 25 Jan 2011 01:08:55 -0000 Received: from nike.apache.org (HELO nike.apache.org) (192.87.106.230) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 25 Jan 2011 01:08:55 +0000 X-ASF-Spam-Status: No, hits=-2000.0 required=10.0 tests=ALL_TRUSTED X-Spam-Check-By: apache.org Received: from [140.211.11.4] (HELO eris.apache.org) (140.211.11.4) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 25 Jan 2011 01:08:52 +0000 Received: by eris.apache.org (Postfix, from userid 65534) id 79D1323889B9; Tue, 25 Jan 2011 01:08:30 +0000 (UTC) Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: svn commit: r1063088 - in /hadoop/mapreduce/trunk: ./ src/java/org/apache/hadoop/mapred/ src/java/org/apache/hadoop/mapreduce/server/tasktracker/ Date: Tue, 25 Jan 2011 01:08:30 -0000 To: mapreduce-commits@hadoop.apache.org From: todd@apache.org X-Mailer: svnmailer-1.0.8 Message-Id: <20110125010830.79D1323889B9@eris.apache.org> X-Virus-Checked: Checked by ClamAV on apache.org Author: todd Date: Tue Jan 25 01:08:29 2011 New Revision: 1063088 URL: http://svn.apache.org/viewvc?rev=1063088&view=rev Log: MAPREDUCE-2238. Fix permissions handling to avoid leaving undeletable directories in local dirs. Contributed by Todd Lipcon Modified: hadoop/mapreduce/trunk/CHANGES.txt hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapred/TaskController.java hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapred/TaskRunner.java hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapred/TaskTracker.java hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapreduce/server/tasktracker/Localizer.java Modified: hadoop/mapreduce/trunk/CHANGES.txt URL: http://svn.apache.org/viewvc/hadoop/mapreduce/trunk/CHANGES.txt?rev=1063088&r1=1063087&r2=1063088&view=diff ============================================================================== --- hadoop/mapreduce/trunk/CHANGES.txt (original) +++ hadoop/mapreduce/trunk/CHANGES.txt Tue Jan 25 01:08:29 2011 @@ -489,6 +489,9 @@ Release 0.22.0 - Unreleased MAPREDUCE-2282. Fix TestMRServerPorts for the changes in TestHDFSServerPorts. (shv via szetszwo) + MAPREDUCE-2238. Fix permissions handling to avoid leaving undeletable directories + in local dirs. (todd) + Release 0.21.1 - Unreleased NEW FEATURES Modified: hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapred/TaskController.java URL: http://svn.apache.org/viewvc/hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapred/TaskController.java?rev=1063088&r1=1063087&r2=1063088&view=diff ============================================================================== --- hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapred/TaskController.java (original) +++ hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapred/TaskController.java Tue Jan 25 01:08:29 2011 @@ -27,9 +27,9 @@ import org.apache.hadoop.conf.Configurab import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.Path; +import org.apache.hadoop.fs.permission.FsPermission; import org.apache.hadoop.mapred.CleanupQueue.PathDeletionContext; import org.apache.hadoop.mapred.JvmManager.JvmEnv; -import org.apache.hadoop.mapreduce.server.tasktracker.Localizer; import org.apache.hadoop.mapreduce.MRConfig; import org.apache.hadoop.util.DiskChecker; import org.apache.hadoop.util.StringUtils; @@ -78,6 +78,8 @@ public abstract class TaskController imp * */ public void setup() throws IOException { + FileSystem localFs = FileSystem.getLocal(conf); + for (String localDir : this.mapredLocalDirs) { // Set up the mapreduce.cluster.local.directories. File mapredlocalDir = new File(localDir); @@ -85,8 +87,8 @@ public abstract class TaskController imp LOG.warn("Unable to create mapreduce.cluster.local.directory : " + mapredlocalDir.getPath()); } else { - Localizer.PermissionsHandler.setPermissions(mapredlocalDir, - Localizer.PermissionsHandler.sevenFiveFive); + localFs.setPermission(new Path(mapredlocalDir.getCanonicalPath()), + new FsPermission((short)0755)); } } @@ -95,8 +97,8 @@ public abstract class TaskController imp if (!taskLog.isDirectory() && !taskLog.mkdirs()) { LOG.warn("Unable to create taskLog directory : " + taskLog.getPath()); } else { - Localizer.PermissionsHandler.setPermissions(taskLog, - Localizer.PermissionsHandler.sevenFiveFive); + localFs.setPermission(new Path(taskLog.getCanonicalPath()), + new FsPermission((short)0755)); } DiskChecker.checkDir(TaskLog.getUserLogDir()); } Modified: hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapred/TaskRunner.java URL: http://svn.apache.org/viewvc/hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapred/TaskRunner.java?rev=1063088&r1=1063087&r2=1063088&view=diff ============================================================================== --- hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapred/TaskRunner.java (original) +++ hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapred/TaskRunner.java Tue Jan 25 01:08:29 2011 @@ -39,12 +39,12 @@ import org.apache.hadoop.mapreduce.filec import org.apache.hadoop.mapreduce.filecache.TaskDistributedCacheManager; import org.apache.hadoop.mapreduce.filecache.TrackerDistributedCacheManager; import org.apache.hadoop.mapreduce.security.TokenCache; -import org.apache.hadoop.mapreduce.server.tasktracker.Localizer; import org.apache.hadoop.fs.FSError; import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.FileUtil; import org.apache.hadoop.fs.LocalDirAllocator; import org.apache.hadoop.fs.Path; +import org.apache.hadoop.fs.permission.FsPermission; import org.apache.hadoop.security.UserGroupInformation; import org.apache.hadoop.util.Shell; import org.apache.hadoop.util.StringUtils; @@ -279,8 +279,9 @@ abstract class TaskRunner extends Thread if (!b) { LOG.warn("mkdirs failed. Ignoring"); } else { - Localizer.PermissionsHandler.setPermissions(logDir, - Localizer.PermissionsHandler.sevenZeroZero); + FileSystem localFs = FileSystem.getLocal(conf); + localFs.setPermission(new Path(logDir.getCanonicalPath()), + new FsPermission((short)0700)); } return logFiles; Modified: hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapred/TaskTracker.java URL: http://svn.apache.org/viewvc/hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapred/TaskTracker.java?rev=1063088&r1=1063087&r2=1063088&view=diff ============================================================================== --- hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapred/TaskTracker.java (original) +++ hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapred/TaskTracker.java Tue Jan 25 01:08:29 2011 @@ -1160,7 +1160,7 @@ public class TaskTracker FileOutputStream out; try { - out = SecureIOUtils.createForWrite(aclFile, 0600); + out = SecureIOUtils.createForWrite(aclFile, 0700); } catch (SecureIOUtils.AlreadyExistsException aee) { LOG.warn("Job ACL file already exists at " + aclFile, aee); return; @@ -1170,8 +1170,6 @@ public class TaskTracker } finally { out.close(); } - Localizer.PermissionsHandler.setPermissions(aclFile, - Localizer.PermissionsHandler.sevenZeroZero); } /** Modified: hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapreduce/server/tasktracker/Localizer.java URL: http://svn.apache.org/viewvc/hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapreduce/server/tasktracker/Localizer.java?rev=1063088&r1=1063087&r2=1063088&view=diff ============================================================================== --- hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapreduce/server/tasktracker/Localizer.java (original) +++ hadoop/mapreduce/trunk/src/java/org/apache/hadoop/mapreduce/server/tasktracker/Localizer.java Tue Jan 25 01:08:29 2011 @@ -30,6 +30,7 @@ import org.apache.hadoop.classification. import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.FileUtil; import org.apache.hadoop.fs.Path; +import org.apache.hadoop.fs.permission.FsPermission; import org.apache.hadoop.mapred.TaskController; import org.apache.hadoop.mapred.TaskLog; import org.apache.hadoop.mapred.TaskTracker; @@ -59,105 +60,6 @@ public class Localizer { taskController = tc; } - @InterfaceAudience.Private - @InterfaceStability.Unstable - public static class PermissionsHandler { - /** - * Permission information useful for setting permissions for a given path. - * Using this, one can set all possible combinations of permissions for the - * owner of the file. But permissions for the group and all others can only - * be set together, i.e. permissions for group cannot be set different from - * those for others and vice versa. - */ - @InterfaceAudience.Private - @InterfaceStability.Unstable - public static class PermissionsInfo { - public boolean readPermissions; - public boolean writePermissions; - public boolean executablePermissions; - public boolean readPermsOwnerOnly; - public boolean writePermsOwnerOnly; - public boolean executePermsOwnerOnly; - - /** - * Create a permissions-info object with the given attributes - * - * @param readPerms - * @param writePerms - * @param executePerms - * @param readOwnerOnly - * @param writeOwnerOnly - * @param executeOwnerOnly - */ - public PermissionsInfo(boolean readPerms, boolean writePerms, - boolean executePerms, boolean readOwnerOnly, boolean writeOwnerOnly, - boolean executeOwnerOnly) { - readPermissions = readPerms; - writePermissions = writePerms; - executablePermissions = executePerms; - readPermsOwnerOnly = readOwnerOnly; - writePermsOwnerOnly = writeOwnerOnly; - executePermsOwnerOnly = executeOwnerOnly; - } - } - - /** - * Set permission on the given file path using the specified permissions - * information. We use java api to set permission instead of spawning chmod - * processes. This saves a lot of time. Using this, one can set all possible - * combinations of permissions for the owner of the file. But permissions - * for the group and all others can only be set together, i.e. permissions - * for group cannot be set different from those for others and vice versa. - * - * This method should satisfy the needs of most of the applications. For - * those it doesn't, {@link FileUtil#chmod} can be used. - * - * @param f file path - * @param pInfo permissions information - * @return true if success, false otherwise - */ - public static boolean setPermissions(File f, PermissionsInfo pInfo) { - if (pInfo == null) { - LOG.debug(" PermissionsInfo is null, returning."); - return true; - } - - LOG.debug("Setting permission for " + f.getAbsolutePath()); - - boolean ret = true; - - // Clear all the flags - ret = f.setReadable(false, false) && ret; - ret = f.setWritable(false, false) && ret; - ret = f.setExecutable(false, false) && ret; - - ret = f.setReadable(pInfo.readPermissions, pInfo.readPermsOwnerOnly); - LOG.debug("Readable status for " + f + " set to " + ret); - ret = - f.setWritable(pInfo.writePermissions, pInfo.writePermsOwnerOnly) - && ret; - LOG.debug("Writable status for " + f + " set to " + ret); - ret = - f.setExecutable(pInfo.executablePermissions, - pInfo.executePermsOwnerOnly) - && ret; - - LOG.debug("Executable status for " + f + " set to " + ret); - return ret; - } - - /** - * Permissions rwxr_xr_x - */ - public static PermissionsInfo sevenFiveFive = - new PermissionsInfo(true, true, true, false, true, false); - /** - * Completely private permissions - */ - public static PermissionsInfo sevenZeroZero = - new PermissionsInfo(true, true, true, true, true, true); - } - // Data-structure for synchronizing localization of user directories. private Map localizedUsers = new HashMap(); @@ -214,35 +116,31 @@ public class Localizer { if (fs.exists(userDir) || fs.mkdirs(userDir)) { // Set permissions on the user-directory - PermissionsHandler.setPermissions( - new File(userDir.toUri().getPath()), - PermissionsHandler.sevenZeroZero); + fs.setPermission(userDir, new FsPermission((short)0700)); userDirStatus = true; // Set up the jobcache directory - File jobCacheDir = - new File(localDir, TaskTracker.getJobCacheSubdir(user)); - if (jobCacheDir.exists() || jobCacheDir.mkdirs()) { + Path jobCacheDir = + new Path(localDir, TaskTracker.getJobCacheSubdir(user)); + if (fs.exists(jobCacheDir) || fs.mkdirs(jobCacheDir)) { // Set permissions on the jobcache-directory - PermissionsHandler.setPermissions(jobCacheDir, - PermissionsHandler.sevenZeroZero); + fs.setPermission(jobCacheDir, new FsPermission((short)0700)); jobCacheDirStatus = true; } else { LOG.warn("Unable to create job cache directory : " - + jobCacheDir.getPath()); + + jobCacheDir); } // Set up the cache directory used for distributed cache files - File distributedCacheDir = - new File(localDir, TaskTracker.getPrivateDistributedCacheDir(user)); - if (distributedCacheDir.exists() || distributedCacheDir.mkdirs()) { + Path distributedCacheDir = + new Path(localDir, TaskTracker.getPrivateDistributedCacheDir(user)); + if (fs.exists(distributedCacheDir) || fs.mkdirs(distributedCacheDir)) { // Set permissions on the distcache-directory - PermissionsHandler.setPermissions(distributedCacheDir, - PermissionsHandler.sevenZeroZero); + fs.setPermission(distributedCacheDir, new FsPermission((short)0700)); distributedCacheDirStatus = true; } else { LOG.warn("Unable to create distributed-cache directory : " - + distributedCacheDir.getPath()); + + distributedCacheDir); } } else { LOG.warn("Unable to create the user directory : " + userDir); @@ -311,8 +209,7 @@ public class Localizer { initJobDirStatus = initJobDirStatus || jobDirStatus; // job-dir has to be private to the TT - Localizer.PermissionsHandler.setPermissions(new File(jobDir.toUri() - .getPath()), Localizer.PermissionsHandler.sevenZeroZero); + fs.setPermission(jobDir, new FsPermission((short)0700)); } if (!initJobDirStatus) { @@ -366,15 +263,11 @@ public class Localizer { * @param jobId */ public void initializeJobLogDir(JobID jobId) throws IOException { - File jobUserLogDir = TaskLog.getJobDir(jobId); - if (!jobUserLogDir.exists()) { - boolean ret = jobUserLogDir.mkdirs(); - if (!ret) { - throw new IOException("Could not create job user log directory: " + - jobUserLogDir); - } + Path jobUserLogDir = new Path(TaskLog.getJobDir(jobId).getCanonicalPath()); + if (!fs.mkdirs(jobUserLogDir)) { + throw new IOException("Could not create job user log directory: " + + jobUserLogDir); } - Localizer.PermissionsHandler.setPermissions(jobUserLogDir, - Localizer.PermissionsHandler.sevenZeroZero); + fs.setPermission(jobUserLogDir, new FsPermission((short)0700)); } }