Return-Path: X-Original-To: apmail-hadoop-common-commits-archive@www.apache.org Delivered-To: apmail-hadoop-common-commits-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 089599AA4 for ; Thu, 2 Feb 2012 17:13:08 +0000 (UTC) Received: (qmail 44492 invoked by uid 500); 2 Feb 2012 17:13:07 -0000 Delivered-To: apmail-hadoop-common-commits-archive@hadoop.apache.org Received: (qmail 44323 invoked by uid 500); 2 Feb 2012 17:13:06 -0000 Mailing-List: contact common-commits-help@hadoop.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: common-dev@hadoop.apache.org Delivered-To: mailing list common-commits@hadoop.apache.org Received: (qmail 44316 invoked by uid 99); 2 Feb 2012 17:13:06 -0000 Received: from athena.apache.org (HELO athena.apache.org) (140.211.11.136) by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 02 Feb 2012 17:13:06 +0000 X-ASF-Spam-Status: No, hits=-2000.0 required=5.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; Thu, 02 Feb 2012 17:13:05 +0000 Received: from eris.apache.org (localhost [127.0.0.1]) by eris.apache.org (Postfix) with ESMTP id 760E72388865; Thu, 2 Feb 2012 17:12:45 +0000 (UTC) Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: svn commit: r1239727 - in /hadoop/common/trunk/hadoop-common-project/hadoop-common: CHANGES.txt src/main/java/org/apache/hadoop/fs/ChecksumFileSystem.java src/test/java/org/apache/hadoop/fs/TestChecksumFileSystem.java Date: Thu, 02 Feb 2012 17:12:45 -0000 To: common-commits@hadoop.apache.org From: bobby@apache.org X-Mailer: svnmailer-1.0.8-patched Message-Id: <20120202171245.760E72388865@eris.apache.org> Author: bobby Date: Thu Feb 2 17:12:44 2012 New Revision: 1239727 URL: http://svn.apache.org/viewvc?rev=1239727&view=rev Log: HADOOP-8001 ChecksumFileSystem's rename doesn't correctly handle checksum files (Daryn Sharp via bobby) Modified: hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/ChecksumFileSystem.java hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestChecksumFileSystem.java Modified: hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt URL: http://svn.apache.org/viewvc/hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt?rev=1239727&r1=1239726&r2=1239727&view=diff ============================================================================== --- hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt (original) +++ hadoop/common/trunk/hadoop-common-project/hadoop-common/CHANGES.txt Thu Feb 2 17:12:44 2012 @@ -81,6 +81,9 @@ Trunk (unreleased changes) 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) Modified: hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/ChecksumFileSystem.java URL: http://svn.apache.org/viewvc/hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/ChecksumFileSystem.java?rev=1239727&r1=1239726&r2=1239727&view=diff ============================================================================== --- hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/ChecksumFileSystem.java (original) +++ hadoop/common/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/ChecksumFileSystem.java Thu Feb 2 17:12:44 2012 @@ -474,18 +474,21 @@ public abstract class ChecksumFileSystem 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; Modified: hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestChecksumFileSystem.java URL: http://svn.apache.org/viewvc/hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestChecksumFileSystem.java?rev=1239727&r1=1239726&r2=1239727&view=diff ============================================================================== --- hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestChecksumFileSystem.java (original) +++ hadoop/common/trunk/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestChecksumFileSystem.java Thu Feb 2 17:12:44 2012 @@ -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))); + } }