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 cn...@apache.org on 2013/07/26 23:13:08 UTC
svn commit: r1507442 - in /hadoop/common/branches/branch-1: CHANGES.txt
src/core/org/apache/hadoop/fs/RawLocalFileSystem.java
src/test/org/apache/hadoop/fs/TestLocalFileSystem.java
Author: cnauroth
Date: Fri Jul 26 21:13:07 2013
New Revision: 1507442
URL: http://svn.apache.org/r1507442
Log:
HADOOP-9507. LocalFileSystem rename() is broken in some cases when destination exists. Contributed by Chris Nauroth.
Modified:
hadoop/common/branches/branch-1/CHANGES.txt
hadoop/common/branches/branch-1/src/core/org/apache/hadoop/fs/RawLocalFileSystem.java
hadoop/common/branches/branch-1/src/test/org/apache/hadoop/fs/TestLocalFileSystem.java
Modified: hadoop/common/branches/branch-1/CHANGES.txt
URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-1/CHANGES.txt?rev=1507442&r1=1507441&r2=1507442&view=diff
==============================================================================
--- hadoop/common/branches/branch-1/CHANGES.txt (original)
+++ hadoop/common/branches/branch-1/CHANGES.txt Fri Jul 26 21:13:07 2013
@@ -102,6 +102,9 @@ Release 1.3.0 - unreleased
MAPREDUCE-5288. ResourceEstimator#getEstimatedTotalMapOutputSize suffers
from divide by zero issues. (kkambatl via tucu)
+ HADOOP-9507. LocalFileSystem rename() is broken in some cases when
+ destination exists. (cnauroth)
+
Release 1.2.1 - 2013.07.06
INCOMPATIBLE CHANGES
Modified: hadoop/common/branches/branch-1/src/core/org/apache/hadoop/fs/RawLocalFileSystem.java
URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-1/src/core/org/apache/hadoop/fs/RawLocalFileSystem.java?rev=1507442&r1=1507441&r2=1507442&view=diff
==============================================================================
--- hadoop/common/branches/branch-1/src/core/org/apache/hadoop/fs/RawLocalFileSystem.java (original)
+++ hadoop/common/branches/branch-1/src/core/org/apache/hadoop/fs/RawLocalFileSystem.java Fri Jul 26 21:13:07 2013
@@ -285,10 +285,35 @@ public class RawLocalFileSystem extends
}
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;
}
- LOG.debug("Falling through to a copy of " + src + " to " + dst);
+
+ // 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.isDir() && 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());
}
Modified: hadoop/common/branches/branch-1/src/test/org/apache/hadoop/fs/TestLocalFileSystem.java
URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-1/src/test/org/apache/hadoop/fs/TestLocalFileSystem.java?rev=1507442&r1=1507441&r2=1507442&view=diff
==============================================================================
--- hadoop/common/branches/branch-1/src/test/org/apache/hadoop/fs/TestLocalFileSystem.java (original)
+++ hadoop/common/branches/branch-1/src/test/org/apache/hadoop/fs/TestLocalFileSystem.java Fri Jul 26 21:13:07 2013
@@ -18,6 +18,7 @@
package org.apache.hadoop.fs;
import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.io.IOUtils;
import org.apache.hadoop.util.StringUtils;
import java.io.*;
import java.util.Arrays;
@@ -259,6 +260,100 @@ public class TestLocalFileSystem extends
stm.close();
}
}
+
+ /**
+ * Tests a simple rename of a directory.
+ */
+ public void testRenameDirectory() throws IOException {
+ FileSystem fs = FileSystem.getLocal(new Configuration());
+ Path src = new Path(TEST_ROOT_DIR, "dir1");
+ Path dst = new Path(TEST_ROOT_DIR, "dir2");
+ try {
+ fs.delete(src, true);
+ fs.delete(dst, true);
+ assertTrue(fs.mkdirs(src));
+ assertTrue(fs.rename(src, dst));
+ assertTrue(fs.exists(dst));
+ assertFalse(fs.exists(src));
+ } finally {
+ cleanupFileSystem(fs, src, dst);
+ }
+ }
+
+ /**
+ * 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
+ */
+ public void testRenameReplaceExistingEmptyDirectory() throws IOException {
+ FileSystem fs = FileSystem.getLocal(new Configuration());
+ Path src = new Path(TEST_ROOT_DIR, "dir1");
+ Path dst = new Path(TEST_ROOT_DIR, "dir2");
+ try {
+ fs.delete(src, true);
+ fs.delete(dst, true);
+ assertTrue(fs.mkdirs(src));
+ writeFile(fs, new Path(src, "file1"));
+ writeFile(fs, new Path(src, "file2"));
+ assertTrue(fs.mkdirs(dst));
+ assertTrue(fs.rename(src, dst));
+ assertTrue(fs.exists(dst));
+ assertTrue(fs.exists(new Path(dst, "file1")));
+ assertTrue(fs.exists(new Path(dst, "file2")));
+ assertFalse(fs.exists(src));
+ } finally {
+ cleanupFileSystem(fs, src, dst);
+ }
+ }
+
+ /**
+ * 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
+ */
+ public void testRenameMoveToExistingNonEmptyDirectory() throws IOException {
+ FileSystem fs = FileSystem.getLocal(new Configuration());
+ Path src = new Path(TEST_ROOT_DIR, "dir1/dir2/dir3");
+ Path dst = new Path(TEST_ROOT_DIR, "dir1");
+ try {
+ fs.delete(src, true);
+ fs.delete(dst, true);
+ assertTrue(fs.mkdirs(src));
+ writeFile(fs, new Path(src, "file1"));
+ writeFile(fs, new Path(src, "file2"));
+ assertTrue(fs.exists(dst));
+ assertTrue(fs.rename(src, dst));
+ assertTrue(fs.exists(dst));
+ assertTrue(fs.exists(new Path(dst, "dir3")));
+ assertTrue(fs.exists(new Path(dst, "dir3/file1")));
+ assertTrue(fs.exists(new Path(dst, "dir3/file2")));
+ assertFalse(fs.exists(src));
+ } finally {
+ cleanupFileSystem(fs, src, dst);
+ }
+ }
private void verifyRead(FSDataInputStream stm, byte[] fileContents,
int seekOff, int toRead) throws IOException {
@@ -275,4 +370,31 @@ public class TestLocalFileSystem extends
fail(s);
}
}
+
+ /**
+ * Cleans up the file system by deleting the given paths and closing the file
+ * system.
+ *
+ * @param fs FileSystem to clean up
+ * @param paths Path... any number of paths to delete
+ */
+ private static void cleanupFileSystem(FileSystem fs, Path... paths) {
+ for (Path path: paths) {
+ deleteQuietly(fs, path);
+ }
+ IOUtils.cleanup(null, fs);
+ }
+
+ /**
+ * Deletes the given path and silences any exceptions.
+ *
+ * @param fs FileSystem to perform the delete
+ * @param path Path to delete
+ */
+ private static void deleteQuietly(FileSystem fs, Path path) {
+ try {
+ fs.delete(path, true);
+ } catch (IOException e) {
+ }
+ }
}