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 2015/04/03 01:16:20 UTC
[1/2] hadoop git commit: HADOOP-9805. Refactor
RawLocalFileSystem#rename for improved testability. Contributed by
Jean-Pierre Matsumoto.
Repository: hadoop
Updated Branches:
refs/heads/branch-2 1278310c2 -> aabd8c8e0
refs/heads/trunk 16b74f978 -> 5763b173d
HADOOP-9805. Refactor RawLocalFileSystem#rename for improved testability. Contributed by Jean-Pierre Matsumoto.
Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo
Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/5763b173
Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/5763b173
Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/5763b173
Branch: refs/heads/trunk
Commit: 5763b173d34dcf7372520076f00b576f493662cd
Parents: 16b74f9
Author: cnauroth <cn...@apache.org>
Authored: Thu Apr 2 16:13:00 2015 -0700
Committer: cnauroth <cn...@apache.org>
Committed: Thu Apr 2 16:13:00 2015 -0700
----------------------------------------------------------------------
hadoop-common-project/hadoop-common/CHANGES.txt | 3 ++
.../apache/hadoop/fs/RawLocalFileSystem.java | 36 ++++++++++-----
.../rawlocal/TestRawlocalContractRename.java | 47 ++++++++++++++++++++
3 files changed, 74 insertions(+), 12 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/hadoop/blob/5763b173/hadoop-common-project/hadoop-common/CHANGES.txt
----------------------------------------------------------------------
diff --git a/hadoop-common-project/hadoop-common/CHANGES.txt b/hadoop-common-project/hadoop-common/CHANGES.txt
index c92e378..260cc83 100644
--- a/hadoop-common-project/hadoop-common/CHANGES.txt
+++ b/hadoop-common-project/hadoop-common/CHANGES.txt
@@ -476,6 +476,9 @@ Release 2.8.0 - UNRELEASED
HADOOP-11660. Add support for hardware crc of HDFS checksums on ARM aarch64
architecture (Edward Nevill via Colin P. McCabe)
+ HADOOP-9805. Refactor RawLocalFileSystem#rename for improved testability.
+ (Jean-Pierre Matsumoto via cnauroth)
+
OPTIMIZATIONS
BUG FIXES
http://git-wip-us.apache.org/repos/asf/hadoop/blob/5763b173/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/RawLocalFileSystem.java
----------------------------------------------------------------------
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 d7866b8..52623b8 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
@@ -43,7 +43,6 @@ import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.fs.permission.FsPermission;
import org.apache.hadoop.io.IOUtils;
import org.apache.hadoop.io.nativeio.NativeIO;
-import org.apache.hadoop.io.nativeio.NativeIOException;
import org.apache.hadoop.util.Progressable;
import org.apache.hadoop.util.Shell;
import org.apache.hadoop.util.StringUtils;
@@ -347,11 +346,29 @@ public class RawLocalFileSystem extends FileSystem {
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.
+ // Else try POSIX style rename on Windows only
+ if (Shell.WINDOWS &&
+ handleEmptyDstDirectoryOnWindows(src, srcFile, dst, 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());
+ }
+
+ @VisibleForTesting
+ public final boolean handleEmptyDstDirectoryOnWindows(Path src, File srcFile,
+ Path dst, File dstFile) throws IOException {
+
+ // 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 renameTo(dstFile) fails. Delete destination and attempt rename
+ // again.
if (this.exists(dst)) {
FileStatus sdst = this.getFileStatus(dst);
if (sdst.isDirectory() && dstFile.list().length == 0) {
@@ -364,12 +381,7 @@ public class RawLocalFileSystem extends FileSystem {
}
}
}
-
- // 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());
+ return false;
}
@Override
http://git-wip-us.apache.org/repos/asf/hadoop/blob/5763b173/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/contract/rawlocal/TestRawlocalContractRename.java
----------------------------------------------------------------------
diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/contract/rawlocal/TestRawlocalContractRename.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/contract/rawlocal/TestRawlocalContractRename.java
index 9e33b34..25611f1 100644
--- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/contract/rawlocal/TestRawlocalContractRename.java
+++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/contract/rawlocal/TestRawlocalContractRename.java
@@ -19,8 +19,13 @@
package org.apache.hadoop.fs.contract.rawlocal;
import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.fs.RawLocalFileSystem;
import org.apache.hadoop.fs.contract.AbstractContractRenameTest;
import org.apache.hadoop.fs.contract.AbstractFSContract;
+import org.apache.hadoop.fs.contract.ContractTestUtils;
+import org.junit.Test;
public class TestRawlocalContractRename extends AbstractContractRenameTest {
@@ -28,5 +33,47 @@ public class TestRawlocalContractRename extends AbstractContractRenameTest {
protected AbstractFSContract createContract(Configuration conf) {
return new RawlocalFSContract(conf);
}
+
+ /**
+ * Test fallback rename code <code>handleEmptyDstDirectoryOnWindows()</code>
+ * even on not Windows platform where the normal <code>File.renameTo()</code>
+ * is supposed to work well. This test has been added for HADOOP-9805.
+ *
+ * @see AbstractContractRenameTest#testRenameWithNonEmptySubDirPOSIX()
+ */
+ @Test
+ public void testRenameWithNonEmptySubDirPOSIX() throws Throwable {
+ final Path renameTestDir = path("testRenameWithNonEmptySubDir");
+ final Path srcDir = new Path(renameTestDir, "src1");
+ final Path srcSubDir = new Path(srcDir, "sub");
+ final Path finalDir = new Path(renameTestDir, "dest");
+ FileSystem fs = getFileSystem();
+ ContractTestUtils.rm(fs, renameTestDir, true, false);
+
+ fs.mkdirs(srcDir);
+ fs.mkdirs(finalDir);
+ ContractTestUtils.writeTextFile(fs, new Path(srcDir, "source.txt"),
+ "this is the file in src dir", false);
+ ContractTestUtils.writeTextFile(fs, new Path(srcSubDir, "subfile.txt"),
+ "this is the file in src/sub dir", false);
+
+ ContractTestUtils.assertPathExists(fs, "not created in src dir",
+ new Path(srcDir, "source.txt"));
+ ContractTestUtils.assertPathExists(fs, "not created in src/sub dir",
+ new Path(srcSubDir, "subfile.txt"));
+
+ RawLocalFileSystem rlfs = (RawLocalFileSystem) fs;
+ rlfs.handleEmptyDstDirectoryOnWindows(srcDir, rlfs.pathToFile(srcDir),
+ finalDir, rlfs.pathToFile(finalDir));
+
+ // Accept only POSIX rename behavior in this test
+ ContractTestUtils.assertPathExists(fs, "not renamed into dest dir",
+ new Path(finalDir, "source.txt"));
+ ContractTestUtils.assertPathExists(fs, "not renamed into dest/sub dir",
+ new Path(finalDir, "sub/subfile.txt"));
+
+ ContractTestUtils.assertPathDoesNotExist(fs, "not deleted",
+ new Path(srcDir, "source.txt"));
+ }
}
[2/2] hadoop git commit: HADOOP-9805. Refactor
RawLocalFileSystem#rename for improved testability. Contributed by
Jean-Pierre Matsumoto.
Posted by cn...@apache.org.
HADOOP-9805. Refactor RawLocalFileSystem#rename for improved testability. Contributed by Jean-Pierre Matsumoto.
(cherry picked from commit 5763b173d34dcf7372520076f00b576f493662cd)
Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo
Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/aabd8c8e
Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/aabd8c8e
Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/aabd8c8e
Branch: refs/heads/branch-2
Commit: aabd8c8e0401f7298d4a12fc749cd795e5ff8d57
Parents: 1278310
Author: cnauroth <cn...@apache.org>
Authored: Thu Apr 2 16:13:00 2015 -0700
Committer: cnauroth <cn...@apache.org>
Committed: Thu Apr 2 16:13:12 2015 -0700
----------------------------------------------------------------------
hadoop-common-project/hadoop-common/CHANGES.txt | 3 ++
.../apache/hadoop/fs/RawLocalFileSystem.java | 36 ++++++++++-----
.../rawlocal/TestRawlocalContractRename.java | 47 ++++++++++++++++++++
3 files changed, 74 insertions(+), 12 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/hadoop/blob/aabd8c8e/hadoop-common-project/hadoop-common/CHANGES.txt
----------------------------------------------------------------------
diff --git a/hadoop-common-project/hadoop-common/CHANGES.txt b/hadoop-common-project/hadoop-common/CHANGES.txt
index d17db41..c0eeb9c 100644
--- a/hadoop-common-project/hadoop-common/CHANGES.txt
+++ b/hadoop-common-project/hadoop-common/CHANGES.txt
@@ -39,6 +39,9 @@ Release 2.8.0 - UNRELEASED
HADOOP-11660. Add support for hardware crc of HDFS checksums on ARM aarch64
architecture (Edward Nevill via Colin P. McCabe)
+ HADOOP-9805. Refactor RawLocalFileSystem#rename for improved testability.
+ (Jean-Pierre Matsumoto via cnauroth)
+
OPTIMIZATIONS
BUG FIXES
http://git-wip-us.apache.org/repos/asf/hadoop/blob/aabd8c8e/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/RawLocalFileSystem.java
----------------------------------------------------------------------
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 0fae8cd..1aea1ba 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
@@ -43,7 +43,6 @@ import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.fs.permission.FsPermission;
import org.apache.hadoop.io.IOUtils;
import org.apache.hadoop.io.nativeio.NativeIO;
-import org.apache.hadoop.io.nativeio.NativeIOException;
import org.apache.hadoop.util.Progressable;
import org.apache.hadoop.util.Shell;
import org.apache.hadoop.util.StringUtils;
@@ -347,11 +346,29 @@ public class RawLocalFileSystem extends FileSystem {
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.
+ // Else try POSIX style rename on Windows only
+ if (Shell.WINDOWS &&
+ handleEmptyDstDirectoryOnWindows(src, srcFile, dst, 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());
+ }
+
+ @VisibleForTesting
+ public final boolean handleEmptyDstDirectoryOnWindows(Path src, File srcFile,
+ Path dst, File dstFile) throws IOException {
+
+ // 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 renameTo(dstFile) fails. Delete destination and attempt rename
+ // again.
if (this.exists(dst)) {
FileStatus sdst = this.getFileStatus(dst);
if (sdst.isDirectory() && dstFile.list().length == 0) {
@@ -364,12 +381,7 @@ public class RawLocalFileSystem extends FileSystem {
}
}
}
-
- // 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());
+ return false;
}
@Override
http://git-wip-us.apache.org/repos/asf/hadoop/blob/aabd8c8e/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/contract/rawlocal/TestRawlocalContractRename.java
----------------------------------------------------------------------
diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/contract/rawlocal/TestRawlocalContractRename.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/contract/rawlocal/TestRawlocalContractRename.java
index 9e33b34..25611f1 100644
--- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/contract/rawlocal/TestRawlocalContractRename.java
+++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/contract/rawlocal/TestRawlocalContractRename.java
@@ -19,8 +19,13 @@
package org.apache.hadoop.fs.contract.rawlocal;
import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.fs.RawLocalFileSystem;
import org.apache.hadoop.fs.contract.AbstractContractRenameTest;
import org.apache.hadoop.fs.contract.AbstractFSContract;
+import org.apache.hadoop.fs.contract.ContractTestUtils;
+import org.junit.Test;
public class TestRawlocalContractRename extends AbstractContractRenameTest {
@@ -28,5 +33,47 @@ public class TestRawlocalContractRename extends AbstractContractRenameTest {
protected AbstractFSContract createContract(Configuration conf) {
return new RawlocalFSContract(conf);
}
+
+ /**
+ * Test fallback rename code <code>handleEmptyDstDirectoryOnWindows()</code>
+ * even on not Windows platform where the normal <code>File.renameTo()</code>
+ * is supposed to work well. This test has been added for HADOOP-9805.
+ *
+ * @see AbstractContractRenameTest#testRenameWithNonEmptySubDirPOSIX()
+ */
+ @Test
+ public void testRenameWithNonEmptySubDirPOSIX() throws Throwable {
+ final Path renameTestDir = path("testRenameWithNonEmptySubDir");
+ final Path srcDir = new Path(renameTestDir, "src1");
+ final Path srcSubDir = new Path(srcDir, "sub");
+ final Path finalDir = new Path(renameTestDir, "dest");
+ FileSystem fs = getFileSystem();
+ ContractTestUtils.rm(fs, renameTestDir, true, false);
+
+ fs.mkdirs(srcDir);
+ fs.mkdirs(finalDir);
+ ContractTestUtils.writeTextFile(fs, new Path(srcDir, "source.txt"),
+ "this is the file in src dir", false);
+ ContractTestUtils.writeTextFile(fs, new Path(srcSubDir, "subfile.txt"),
+ "this is the file in src/sub dir", false);
+
+ ContractTestUtils.assertPathExists(fs, "not created in src dir",
+ new Path(srcDir, "source.txt"));
+ ContractTestUtils.assertPathExists(fs, "not created in src/sub dir",
+ new Path(srcSubDir, "subfile.txt"));
+
+ RawLocalFileSystem rlfs = (RawLocalFileSystem) fs;
+ rlfs.handleEmptyDstDirectoryOnWindows(srcDir, rlfs.pathToFile(srcDir),
+ finalDir, rlfs.pathToFile(finalDir));
+
+ // Accept only POSIX rename behavior in this test
+ ContractTestUtils.assertPathExists(fs, "not renamed into dest dir",
+ new Path(finalDir, "source.txt"));
+ ContractTestUtils.assertPathExists(fs, "not renamed into dest/sub dir",
+ new Path(finalDir, "sub/subfile.txt"));
+
+ ContractTestUtils.assertPathDoesNotExist(fs, "not deleted",
+ new Path(srcDir, "source.txt"));
+ }
}