HDFS-3922. namenode throws away blocks under construction on restart (Kihwal Lee via daryn)

git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/branches/branch-0.23@1391150 13f79535-47bb-0310-9956-ffa450edef68

hadoop-common-project/hadoop-common/CHANGES.txt

@@ -224,6 +224,9 @@ Release 0.23.3 - UNRELEASED
     HADOOP-8727. Gracefully deprecate dfs.umaskmode in 2.x onwards (Harsh J
     via bobby)
 
+    HDFS-3922. namenode throws away blocks under construction on restart
+    (Kihwal Lee via daryn)
+
 Release 0.23.2 - UNRELEASED 
 
   NEW FEATURES

hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogLoader.java

@@ -202,6 +202,8 @@ public class FSEditLogLoader {
                   addCloseOp.path, permissions, blocks, replication,
                   addCloseOp.mtime, addCloseOp.atime, blockSize,
                   addCloseOp.clientName, addCloseOp.clientMachine);
+              fsNamesys.leaseManager.addLease(addCloseOp.clientName, 
+                  addCloseOp.path);
             } else {
               fsDir.updateFile(oldFile, addCloseOp.path, blocks,
                   addCloseOp.mtime, addCloseOp.atime);
@@ -221,28 +223,14 @@ public class FSEditLogLoader {
                   INodeFile newFile =
                     ((INodeFileUnderConstruction)oldFile).convertToInodeFile();
                   fsDir.replaceNode(addCloseOp.path, oldFile, newFile);
+                  fsNamesys.leaseManager.removeLease(
+                      ((INodeFileUnderConstruction)oldFile).getClientName(),
+                      addCloseOp.path);
                 }
               } else if(! oldFile.isUnderConstruction()) {  // OP_ADD for append
-                INodeFileUnderConstruction cons = new INodeFileUnderConstruction(
-                    oldFile.getLocalNameBytes(),
-                    oldFile.getReplication(), 
-                    oldFile.getModificationTime(),
-                    oldFile.getPreferredBlockSize(),
-                    oldFile.getBlocks(),
-                    oldFile.getPermissionStatus(),
-                    addCloseOp.clientName,
-                    addCloseOp.clientMachine,
-                    null);
-                fsDir.replaceNode(addCloseOp.path, oldFile, cons);
-              }
-            }
-            // Update file lease
-            if(addCloseOp.opCode == FSEditLogOpCodes.OP_ADD) {
-              fsNamesys.leaseManager.addLease(addCloseOp.clientName, addCloseOp.path);
-            } else {  // Ops.OP_CLOSE
-              if (oldFile.isUnderConstruction()) {
-                fsNamesys.leaseManager.removeLease(
-                    ((INodeFileUnderConstruction)oldFile).getClientName(), addCloseOp.path);
+                fsNamesys.prepareFileForWrite(addCloseOp.path, oldFile,
+                    addCloseOp.clientName, addCloseOp.clientMachine, null,
+                    false);
               }
             }
             break;

hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java

@@ -1199,7 +1199,7 @@ public class FSNamesystem implements Namesystem, FSClusterStats,
     }
 
     try {
-      INode myFile = dir.getFileINode(src);
+      INodeFile myFile = dir.getFileINode(src);
       recoverLeaseInternal(myFile, src, holder, clientMachine, false);
 
       try {
@@ -1228,30 +1228,8 @@ public class FSNamesystem implements Namesystem, FSClusterStats,
           blockManager.getDatanodeManager().getDatanodeByHost(clientMachine);
 
       if (append && myFile != null) {
-        //
-        // Replace current node with a INodeUnderConstruction.
-        // Recreate in-memory lease record.
-        //
-        INodeFile node = (INodeFile) myFile;
-        INodeFileUnderConstruction cons = new INodeFileUnderConstruction(
-                                        node.getLocalNameBytes(),
-                                        node.getReplication(),
-                                        node.getModificationTime(),
-                                        node.getPreferredBlockSize(),
-                                        node.getBlocks(),
-                                        node.getPermissionStatus(),
-                                        holder,
-                                        clientMachine,
-                                        clientNode);
-        dir.replaceNode(src, node, cons);
-        leaseManager.addLease(cons.getClientName(), src);
-        
-        // convert last block to under-construction
-        LocatedBlock ret = blockManager.convertLastBlockToUnderConstruction(cons);
-
-        // add append file record to log, record lease, etc.
-        getEditLog().logOpenFile(src, cons);
-        return ret;
+        return prepareFileForWrite(
+            src, myFile, holder, clientMachine, clientNode, true);
       } else {
        // Now we can add the name to the filesystem. This file has no
        // blocks associated with it.
@@ -1283,6 +1261,43 @@ public class FSNamesystem implements Namesystem, FSClusterStats,
     return null;
   }
 
+  /**
+   * Replace current node with a INodeUnderConstruction.
+   * Recreate in-memory lease record.
+   * 
+   * @param src path to the file
+   * @param file existing file object
+   * @param leaseHolder identifier of the lease holder on this file
+   * @param clientMachine identifier of the client machine
+   * @param clientNode if the client is collocated with a DN, that DN's descriptor
+   * @param writeToEditLog whether to persist this change to the edit log
+   * @return the last block locations if the block is partial or null otherwise
+   * @throws UnresolvedLinkException
+   * @throws IOException
+   */
+  LocatedBlock prepareFileForWrite(String src, INodeFile file,
+      String leaseHolder, String clientMachine, DatanodeDescriptor clientNode,
+      boolean writeToEditLog) throws IOException {
+    INodeFileUnderConstruction cons = new INodeFileUnderConstruction(
+                                    file.getLocalNameBytes(),
+                                    file.getReplication(),
+                                    file.getModificationTime(),
+                                    file.getPreferredBlockSize(),
+                                    file.getBlocks(),
+                                    file.getPermissionStatus(),
+                                    leaseHolder,
+                                    clientMachine,
+                                    clientNode);
+    dir.replaceNode(src, file, cons);
+    leaseManager.addLease(cons.getClientName(), src);
+
+    LocatedBlock ret = blockManager.convertLastBlockToUnderConstruction(cons);
+    if (writeToEditLog) {
+      getEditLog().logOpenFile(src, cons);
+    }
+    return ret;
+  }
+
   /**
    * Recover lease;
    * Immediately revoke the lease of the current lease holder and start lease

hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFSEditLogLoader.java

@@ -33,6 +33,7 @@ import java.util.SortedMap;
 import org.apache.commons.logging.impl.Log4JLogger;
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.FSDataOutputStream;
 import org.apache.hadoop.fs.Path;
 import org.apache.hadoop.hdfs.DFSConfigKeys;
 import org.apache.hadoop.hdfs.DFSTestUtil;
@@ -147,6 +148,57 @@ public class TestFSEditLogLoader {
       }
     }
   }
+
+  @Test(timeout=30000)
+  public void testBlocksUnderConstruction() throws IOException {
+    // start a cluster 
+    Configuration conf = new HdfsConfiguration();
+    conf.setBoolean(DFSConfigKeys.DFS_SUPPORT_APPEND_KEY, true);
+    conf.setInt(DFSConfigKeys.DFS_NAMENODE_SAFEMODE_THRESHOLD_PCT_KEY, 1);
+    MiniDFSCluster cluster = null;
+    FSDataOutputStream out = null;
+    try {
+      cluster = new MiniDFSCluster.Builder(conf).numDataNodes(1)
+          .build();
+      cluster.waitActive();
+      FileSystem fs = cluster.getFileSystem();
+
+      Path p = new Path("/testfile1");
+      DFSTestUtil.createFile(fs, p, 10, (short)1, 1);
+
+      // Reopen for append. The block will become RBW.
+      out = fs.append(p);
+      out.write(1234);
+      out.hflush();
+
+      // Shutdown without finalizing the last block. It will become
+      // RWR in the initial block report.
+      cluster.shutdown();
+      cluster = null;
+      try {
+        out.close();
+      } catch (IOException ioe) { }
+
+      try {
+         fs.close();
+      } catch (IOException ioe) { }
+
+      cluster = new MiniDFSCluster.Builder(conf).numDataNodes(1)
+          .format(false).build();
+      cluster.waitActive();
+
+      // If NN thinks there is no block under construction, the 
+      // reported RWR block will be thrown away and it will never
+      // get out of safe mode.  This should not happen.
+      cluster.shutdown();
+      cluster = null;
+    } finally {
+      if (cluster != null) {
+        cluster.shutdown();
+      }
+    }
+  }
+
   
   /**
    * Test that the valid number of transactions can be counted from a file.