diff --git a/hadoop-common-project/hadoop-common/CHANGES.txt b/hadoop-common-project/hadoop-common/CHANGES.txt index 80ef6f41614..bba8cd83cbc 100644 --- a/hadoop-common-project/hadoop-common/CHANGES.txt +++ b/hadoop-common-project/hadoop-common/CHANGES.txt @@ -382,6 +382,9 @@ Release 2.1.0-beta - 2013-07-02 HADOOP-9773. TestLightWeightCache should not set size limit to zero when testing it. (szetszwo) + HADOOP-9507. LocalFileSystem rename() is broken in some cases when + destination exists. (cnauroth) + BREAKDOWN OF HADOOP-8562 SUBTASKS AND RELATED JIRAS HADOOP-8924. Hadoop Common creating package-info.java must not depend on diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/RawLocalFileSystem.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/RawLocalFileSystem.java index 9e23f859cfe..5d02ff4b3bc 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/RawLocalFileSystem.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/RawLocalFileSystem.java @@ -319,9 +319,35 @@ public FSDataOutputStream createNonRecursive(Path f, FsPermission permission, @Override public boolean rename(Path src, Path dst) throws IOException { - if (pathToFile(src).renameTo(pathToFile(dst))) { + // Attempt rename using Java API. + File srcFile = pathToFile(src); + File dstFile = pathToFile(dst); + if (srcFile.renameTo(dstFile)) { return true; } + + // Enforce POSIX rename behavior that a source directory replaces an existing + // destination if the destination is an empty directory. On most platforms, + // this is already handled by the Java API call above. Some platforms + // (notably Windows) do not provide this behavior, so the Java API call above + // fails. Delete destination and attempt rename again. + if (this.exists(dst)) { + FileStatus sdst = this.getFileStatus(dst); + if (sdst.isDirectory() && dstFile.list().length == 0) { + if (LOG.isDebugEnabled()) { + LOG.debug("Deleting empty destination and renaming " + src + " to " + + dst); + } + if (this.delete(dst, false) && srcFile.renameTo(dstFile)) { + return true; + } + } + } + + // The fallback behavior accomplishes the rename by a full copy. + if (LOG.isDebugEnabled()) { + LOG.debug("Falling through to a copy of " + src + " to " + dst); + } return FileUtil.copy(this, src, this, dst, true, getConf()); } diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestLocalFileSystem.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestLocalFileSystem.java index bef4d86ec6f..255a592a058 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestLocalFileSystem.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestLocalFileSystem.java @@ -417,6 +417,88 @@ public void testBufferedFSInputStream() throws IOException { stm.close(); } } + + /** + * Tests a simple rename of a directory. + */ + @Test + public void testRenameDirectory() throws IOException { + Path src = new Path(TEST_ROOT_DIR, "dir1"); + Path dst = new Path(TEST_ROOT_DIR, "dir2"); + fileSys.delete(src, true); + fileSys.delete(dst, true); + assertTrue(fileSys.mkdirs(src)); + assertTrue(fileSys.rename(src, dst)); + assertTrue(fileSys.exists(dst)); + assertFalse(fileSys.exists(src)); + } + + /** + * Tests that renaming a directory replaces the destination if the destination + * is an existing empty directory. + * + * Before: + * /dir1 + * /file1 + * /file2 + * /dir2 + * + * After rename("/dir1", "/dir2"): + * /dir2 + * /file1 + * /file2 + */ + @Test + public void testRenameReplaceExistingEmptyDirectory() throws IOException { + Path src = new Path(TEST_ROOT_DIR, "dir1"); + Path dst = new Path(TEST_ROOT_DIR, "dir2"); + fileSys.delete(src, true); + fileSys.delete(dst, true); + assertTrue(fileSys.mkdirs(src)); + writeFile(fileSys, new Path(src, "file1"), 1); + writeFile(fileSys, new Path(src, "file2"), 1); + assertTrue(fileSys.mkdirs(dst)); + assertTrue(fileSys.rename(src, dst)); + assertTrue(fileSys.exists(dst)); + assertTrue(fileSys.exists(new Path(dst, "file1"))); + assertTrue(fileSys.exists(new Path(dst, "file2"))); + assertFalse(fileSys.exists(src)); + } + + /** + * Tests that renaming a directory to an existing directory that is not empty + * results in a full copy of source to destination. + * + * Before: + * /dir1 + * /dir2 + * /dir3 + * /file1 + * /file2 + * + * After rename("/dir1/dir2/dir3", "/dir1"): + * /dir1 + * /dir3 + * /file1 + * /file2 + */ + @Test + public void testRenameMoveToExistingNonEmptyDirectory() throws IOException { + Path src = new Path(TEST_ROOT_DIR, "dir1/dir2/dir3"); + Path dst = new Path(TEST_ROOT_DIR, "dir1"); + fileSys.delete(src, true); + fileSys.delete(dst, true); + assertTrue(fileSys.mkdirs(src)); + writeFile(fileSys, new Path(src, "file1"), 1); + writeFile(fileSys, new Path(src, "file2"), 1); + assertTrue(fileSys.exists(dst)); + assertTrue(fileSys.rename(src, dst)); + assertTrue(fileSys.exists(dst)); + assertTrue(fileSys.exists(new Path(dst, "dir3"))); + assertTrue(fileSys.exists(new Path(dst, "dir3/file1"))); + assertTrue(fileSys.exists(new Path(dst, "dir3/file2"))); + assertFalse(fileSys.exists(src)); + } private void verifyRead(FSDataInputStream stm, byte[] fileContents, int seekOff, int toRead) throws IOException {