From 7c0614e301f483cd852c5916c565ef84e5d211a4 Mon Sep 17 00:00:00 2001 From: Robert Joseph Evans Date: Thu, 2 Feb 2012 17:19:26 +0000 Subject: [PATCH] svn merge -c 1239727 FIXES HADOOP-8001 git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/branches/branch-0.23@1239730 13f79535-47bb-0310-9956-ffa450edef68 --- .../hadoop-common/CHANGES.txt | 2 + .../apache/hadoop/fs/ChecksumFileSystem.java | 17 +++--- .../hadoop/fs/TestChecksumFileSystem.java | 54 +++++++++++++++++++ 3 files changed, 66 insertions(+), 7 deletions(-) diff --git a/hadoop-common-project/hadoop-common/CHANGES.txt b/hadoop-common-project/hadoop-common/CHANGES.txt index 9ec5911a917..1ff2f3fa963 100644 --- a/hadoop-common-project/hadoop-common/CHANGES.txt +++ b/hadoop-common-project/hadoop-common/CHANGES.txt @@ -95,6 +95,8 @@ Release 0.23.1 - Unreleased OPTIMIZATIONS BUG FIXES + HADOOP-8001 ChecksumFileSystem's rename doesn't correctly handle checksum + files. (Daryn Sharp via bobby) HADOOP-8006 TestFSInputChecker is failing in trunk. (Daryn Sharp via bobby) diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/ChecksumFileSystem.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/ChecksumFileSystem.java index 040f59dbb8c..e122f25dcdd 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/ChecksumFileSystem.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/ChecksumFileSystem.java @@ -474,18 +474,21 @@ public abstract class ChecksumFileSystem extends FilterFileSystem { if (fs.isDirectory(src)) { return fs.rename(src, dst); } else { + if (fs.isDirectory(dst)) { + dst = new Path(dst, src.getName()); + } boolean value = fs.rename(src, dst); if (!value) return false; - Path checkFile = getChecksumFile(src); - if (fs.exists(checkFile)) { //try to rename checksum - if (fs.isDirectory(dst)) { - value = fs.rename(checkFile, dst); - } else { - value = fs.rename(checkFile, getChecksumFile(dst)); - } + Path srcCheckFile = getChecksumFile(src); + Path dstCheckFile = getChecksumFile(dst); + if (fs.exists(srcCheckFile)) { //try to rename checksum + value = fs.rename(srcCheckFile, dstCheckFile); + } else if (fs.exists(dstCheckFile)) { + // no src checksum, so remove dst checksum + value = fs.delete(dstCheckFile, true); } return value; diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestChecksumFileSystem.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestChecksumFileSystem.java index 80347a72b45..0c24ad59b69 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestChecksumFileSystem.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestChecksumFileSystem.java @@ -203,4 +203,58 @@ public class TestChecksumFileSystem { String str = readFile(localFs, testPath, 1024); assertEquals("testing stale checksum", str); } + + @Test + public void testRenameFileToFile() throws Exception { + Path srcPath = new Path(TEST_ROOT_DIR, "testRenameSrc"); + Path dstPath = new Path(TEST_ROOT_DIR, "testRenameDst"); + verifyRename(srcPath, dstPath, false); + } + + @Test + public void testRenameFileIntoDir() throws Exception { + Path srcPath = new Path(TEST_ROOT_DIR, "testRenameSrc"); + Path dstPath = new Path(TEST_ROOT_DIR, "testRenameDir"); + localFs.mkdirs(dstPath); + verifyRename(srcPath, dstPath, true); + } + + @Test + public void testRenameFileIntoDirFile() throws Exception { + Path srcPath = new Path(TEST_ROOT_DIR, "testRenameSrc"); + Path dstPath = new Path(TEST_ROOT_DIR, "testRenameDir/testRenameDst"); + assertTrue(localFs.mkdirs(dstPath)); + verifyRename(srcPath, dstPath, false); + } + + + void verifyRename(Path srcPath, Path dstPath, boolean dstIsDir) + throws Exception { + localFs.delete(srcPath,true); + localFs.delete(dstPath,true); + + Path realDstPath = dstPath; + if (dstIsDir) { + localFs.mkdirs(dstPath); + realDstPath = new Path(dstPath, srcPath.getName()); + } + + // ensure file + checksum are moved + writeFile(localFs, srcPath, 1); + assertTrue(localFs.exists(localFs.getChecksumFile(srcPath))); + assertTrue(localFs.rename(srcPath, dstPath)); + assertTrue(localFs.exists(localFs.getChecksumFile(realDstPath))); + + // create a file with no checksum, rename, ensure dst checksum is removed + writeFile(localFs.getRawFileSystem(), srcPath, 1); + assertFalse(localFs.exists(localFs.getChecksumFile(srcPath))); + assertTrue(localFs.rename(srcPath, dstPath)); + assertFalse(localFs.exists(localFs.getChecksumFile(realDstPath))); + + // create file with checksum, rename over prior dst with no checksum + writeFile(localFs, srcPath, 1); + assertTrue(localFs.exists(localFs.getChecksumFile(srcPath))); + assertTrue(localFs.rename(srcPath, dstPath)); + assertTrue(localFs.exists(localFs.getChecksumFile(realDstPath))); + } }