diff --git a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt index 714f3ba8409..92b1dc8c811 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt +++ b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt @@ -810,6 +810,9 @@ Release 2.5.0 - UNRELEASED HDFS-6631. TestPread#testHedgedReadLoopTooManyTimes fails intermittently. (Liang Xie via cnauroth) + HDFS-6647. Edit log corruption when pipeline recovery occurs for deleted + file present in snapshot (kihwal) + BREAKDOWN OF HDFS-2006 SUBTASKS AND RELATED JIRAS HDFS-6299. Protobuf for XAttr and client-side implementation. (Yi Liu via umamahesh) diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java index a6aa05e0ebd..2fe5bad0c4b 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java @@ -3101,10 +3101,9 @@ private INodeFile checkLease(String src, String holder, INode inode, : "Holder " + holder + " does not have any open files.")); } // No further modification is allowed on a deleted file. - // A file is considered deleted, if it has no parent or is marked + // A file is considered deleted, if it is not in the inodeMap or is marked // as deleted in the snapshot feature. - if (file.getParent() == null || (file.isWithSnapshot() && - file.getFileWithSnapshotFeature().isCurrentFileDeleted())) { + if (isFileDeleted(file)) { throw new FileNotFoundException(src); } String clientName = file.getFileUnderConstructionFeature().getClientName(); @@ -6251,6 +6250,16 @@ private long nextBlockId() throws IOException { return blockId; } + private boolean isFileDeleted(INodeFile file) { + // Not in the inodeMap or in the snapshot but marked deleted. + if (dir.getInode(file.getId()) == null || + file.getParent() == null || (file.isWithSnapshot() && + file.getFileWithSnapshotFeature().isCurrentFileDeleted())) { + return true; + } + return false; + } + private INodeFile checkUCBlock(ExtendedBlock block, String clientName) throws IOException { assert hasWriteLock(); @@ -6267,7 +6276,7 @@ private INodeFile checkUCBlock(ExtendedBlock block, // check file inode final INodeFile file = ((INode)storedBlock.getBlockCollection()).asFile(); - if (file == null || !file.isUnderConstruction()) { + if (file == null || !file.isUnderConstruction() || isFileDeleted(file)) { throw new IOException("The file " + storedBlock + " belonged to does not exist or it is not under construction."); } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestUpdatePipelineWithSnapshots.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestUpdatePipelineWithSnapshots.java new file mode 100644 index 00000000000..c8955fa2f3d --- /dev/null +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/snapshot/TestUpdatePipelineWithSnapshots.java @@ -0,0 +1,109 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.hadoop.hdfs.server.namenode.snapshot; + +import java.io.IOException; + +import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.fs.FSDataInputStream; +import org.apache.hadoop.fs.FileSystem; +import org.apache.hadoop.fs.Path; +import org.apache.hadoop.hdfs.DFSOutputStream; +import org.apache.hadoop.hdfs.DFSTestUtil; +import org.apache.hadoop.hdfs.DistributedFileSystem; +import org.apache.hadoop.hdfs.HdfsConfiguration; +import org.apache.hadoop.hdfs.MiniDFSCluster; +import org.apache.hadoop.hdfs.protocol.ExtendedBlock; +import org.apache.hadoop.hdfs.protocol.LocatedBlock; +import org.apache.hadoop.hdfs.server.protocol.NamenodeProtocols; +import org.apache.hadoop.io.IOUtils; +import static org.apache.hadoop.test.GenericTestUtils.assertExceptionContains; +import org.junit.Test; + +public class TestUpdatePipelineWithSnapshots { + + // Regression test for HDFS-6647. + @Test + public void testUpdatePipelineAfterDelete() throws Exception { + Configuration conf = new HdfsConfiguration(); + Path file = new Path("/test-file"); + MiniDFSCluster cluster = new MiniDFSCluster.Builder(conf).build(); + + try { + FileSystem fs = cluster.getFileSystem(); + NamenodeProtocols namenode = cluster.getNameNodeRpc(); + DFSOutputStream out = null; + try { + // Create a file and make sure a block is allocated for it. + out = (DFSOutputStream)(fs.create(file). + getWrappedStream()); + out.write(1); + out.hflush(); + + // Create a snapshot that includes the file. + SnapshotTestHelper.createSnapshot((DistributedFileSystem) fs, + new Path("/"), "s1"); + + // Grab the block info of this file for later use. + FSDataInputStream in = null; + ExtendedBlock oldBlock = null; + try { + in = fs.open(file); + oldBlock = DFSTestUtil.getAllBlocks(in).get(0).getBlock(); + } finally { + IOUtils.closeStream(in); + } + + // Allocate a new block ID/gen stamp so we can simulate pipeline + // recovery. + String clientName = ((DistributedFileSystem)fs).getClient() + .getClientName(); + LocatedBlock newLocatedBlock = namenode.updateBlockForPipeline( + oldBlock, clientName); + ExtendedBlock newBlock = new ExtendedBlock(oldBlock.getBlockPoolId(), + oldBlock.getBlockId(), oldBlock.getNumBytes(), + newLocatedBlock.getBlock().getGenerationStamp()); + + // Delete the file from the present FS. It will still exist the + // previously-created snapshot. This will log an OP_DELETE for the + // file in question. + fs.delete(file, true); + + // Simulate a pipeline recovery, wherein a new block is allocated + // for the existing block, resulting in an OP_UPDATE_BLOCKS being + // logged for the file in question. + try { + namenode.updatePipeline(clientName, oldBlock, newBlock, + newLocatedBlock.getLocations(), newLocatedBlock.getStorageIDs()); + } catch (IOException ioe) { + // normal + assertExceptionContains( + "does not exist or it is not under construction", ioe); + } + + // Make sure the NN can restart with the edit logs as we have them now. + cluster.restartNameNode(true); + } finally { + IOUtils.closeStream(out); + } + } finally { + cluster.shutdown(); + } + } + +}