HDFS-132. Port to branch 0.20. Contributed by Konstantin Shvachko.

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

CHANGES.txt

@@ -39,6 +39,9 @@ Release 0.20.3 - Unreleased
     HDFS-1258. Clearing namespace quota on "/" corrupts fs image.  
     (Aaron T. Myers via szetszwo)
 
+    HDFS-132. Fix namenode to not report files deleted metrics for deletions
+    done while replaying edits during startup. (suresh & shv)
+
   IMPROVEMENTS
 
     MAPREDUCE-1407. Update javadoc in mapreduce.{Mapper,Reducer} to match

src/hdfs/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java

@@ -25,9 +25,6 @@ import org.apache.hadoop.fs.FileStatus;
 import org.apache.hadoop.fs.Path;
 import org.apache.hadoop.fs.ContentSummary;
 import org.apache.hadoop.fs.permission.*;
-import org.apache.hadoop.metrics.MetricsRecord;
-import org.apache.hadoop.metrics.MetricsUtil;
-import org.apache.hadoop.metrics.MetricsContext;
 import org.apache.hadoop.hdfs.protocol.FSConstants;
 import org.apache.hadoop.hdfs.protocol.Block;
 import org.apache.hadoop.hdfs.protocol.QuotaExceededException;
@@ -49,8 +46,6 @@ class FSDirectory implements FSConstants, Closeable {
   final INodeDirectoryWithQuota rootDir;
   FSImage fsImage;  
   private boolean ready = false;
-  // Metrics record
-  private MetricsRecord directoryMetrics = null;
 
   /** Access an existing dfs name directory. */
   FSDirectory(FSNamesystem ns, Configuration conf) {
@@ -65,13 +60,6 @@ class FSDirectory implements FSConstants, Closeable {
         Integer.MAX_VALUE, -1);
     this.fsImage = fsImage;
     namesystem = ns;
-    initialize(conf);
-  }
-    
-  private void initialize(Configuration conf) {
-    MetricsContext metricsContext = MetricsUtil.getContext("dfs");
-    directoryMetrics = MetricsUtil.createRecord(metricsContext, "FSDirectory");
-    directoryMetrics.setTag("sessionId", conf.get("session.id"));
   }
 
   void loadFSImage(Collection<File> dataDirs,
@@ -103,8 +91,8 @@ class FSDirectory implements FSConstants, Closeable {
   }
 
   private void incrDeletedFileCount(int count) {
-    directoryMetrics.incrMetric("files_deleted", count);
-    directoryMetrics.update();
+    if (namesystem != null)
+      NameNode.getNameNodeMetrics().numFilesDeleted.inc(count);
   }
     
   /**
@@ -569,17 +557,19 @@ class FSDirectory implements FSConstants, Closeable {
   /**
    * Remove the file from management, return blocks
    */
-  INode delete(String src) {
+  boolean delete(String src) {
     if (NameNode.stateChangeLog.isDebugEnabled()) {
       NameNode.stateChangeLog.debug("DIR* FSDirectory.delete: "+src);
     }
     waitForReady();
     long now = FSNamesystem.now();
-    INode deletedNode = unprotectedDelete(src, now);
-    if (deletedNode != null) {
-      fsImage.getEditLog().logDelete(src, now);
+    int filesRemoved = unprotectedDelete(src, now);
+    if (filesRemoved <= 0) {
+      return false;
     }
-    return deletedNode;
+    incrDeletedFileCount(filesRemoved);
+    fsImage.getEditLog().logDelete(src, now);
+    return true;
   }
   
   /** Return if a directory is empty or not **/
@@ -604,9 +594,9 @@ class FSDirectory implements FSConstants, Closeable {
    * @param src a string representation of a path to an inode
    * @param modificationTime the time the inode is removed
    * @param deletedBlocks the place holder for the blocks to be removed
-   * @return if the deletion succeeds
+   * @return the number of inodes deleted; 0 if no inodes are deleted.
    */ 
-  INode unprotectedDelete(String src, long modificationTime) {
+  int unprotectedDelete(String src, long modificationTime) {
     src = normalizePath(src);
 
     synchronized (rootDir) {
@@ -616,12 +606,12 @@ class FSDirectory implements FSConstants, Closeable {
       if (targetNode == null) { // non-existent src
         NameNode.stateChangeLog.debug("DIR* FSDirectory.unprotectedDelete: "
             +"failed to remove "+src+" because it does not exist");
-        return null;
+        return 0;
       } else if (inodes.length == 1) { // src is the root
         NameNode.stateChangeLog.warn("DIR* FSDirectory.unprotectedDelete: " +
             "failed to remove " + src +
             " because the root is not allowed to be deleted");
-        return null;
+        return 0;
       } else {
         try {
           // Remove the node from the namespace
@@ -631,17 +621,16 @@ class FSDirectory implements FSConstants, Closeable {
           // GC all the blocks underneath the node.
           ArrayList<Block> v = new ArrayList<Block>();
           int filesRemoved = targetNode.collectSubtreeBlocksAndClear(v);
-          incrDeletedFileCount(filesRemoved);
           namesystem.removePathAndBlocks(src, v);
           if (NameNode.stateChangeLog.isDebugEnabled()) {
             NameNode.stateChangeLog.debug("DIR* FSDirectory.unprotectedDelete: "
               +src+" is removed");
           }
-          return targetNode;
+          return filesRemoved;
         } catch (IOException e) {
           NameNode.stateChangeLog.warn("DIR* FSDirectory.unprotectedDelete: " +
               "failed to remove " + src + " because " + e.getMessage());
-          return null;
+          return 0;
         }
       }
     }
@@ -940,7 +929,7 @@ class FSDirectory implements FSConstants, Closeable {
           return false;
         }
         // Directory creation also count towards FilesCreated
-        // to match count of files_deleted metric. 
+        // to match count of FilesDeleted metric. 
         if (namesystem != null)
           NameNode.getNameNodeMetrics().numFilesCreated.inc();
         fsImage.getEditLog().logMkDir(cur, inodes[i]);
@@ -1162,7 +1151,6 @@ class FSDirectory implements FSConstants, Closeable {
    * @param dir the root of the tree that represents the directory
    * @param counters counters for name space and disk space
    * @param nodesInPath INodes for the each of components in the path.
-   * @return the size of the tree
    */
   private static void updateCountForINodeWithQuota(INodeDirectory dir, 
                                                INode.DirCounts counts,

src/hdfs/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java

@@ -1702,7 +1702,7 @@ public class FSNamesystem implements FSConstants, FSNamesystemMBean {
       checkPermission(src, false, null, FsAction.WRITE, null, FsAction.ALL);
     }
 
-    return dir.delete(src) != null;
+    return dir.delete(src);
   }
 
   void removePathAndBlocks(String src, List<Block> blocks) throws IOException {

src/hdfs/org/apache/hadoop/hdfs/server/namenode/metrics/NameNodeMetrics.java

@@ -48,6 +48,8 @@ public class NameNodeMetrics implements Updater {
     
     private NameNodeActivtyMBean namenodeActivityMBean;
     
+    public MetricsTimeVaryingInt numCreateFileOps = 
+                    new MetricsTimeVaryingInt("CreateFileOps", registry);
     public MetricsTimeVaryingInt numFilesCreated =
                           new MetricsTimeVaryingInt("FilesCreated", registry);
     public MetricsTimeVaryingInt numFilesAppended =
@@ -58,21 +60,23 @@ public class NameNodeMetrics implements Updater {
                     new MetricsTimeVaryingInt("FilesRenamed", registry);
     public MetricsTimeVaryingInt numGetListingOps = 
                     new MetricsTimeVaryingInt("GetListingOps", registry);
-    public MetricsTimeVaryingInt numCreateFileOps = 
-                    new MetricsTimeVaryingInt("CreateFileOps", registry);
     public MetricsTimeVaryingInt numDeleteFileOps = 
                           new MetricsTimeVaryingInt("DeleteFileOps", registry);
+    public MetricsTimeVaryingInt numFilesDeleted = new MetricsTimeVaryingInt(
+        "FilesDeleted", registry, 
+        "Number of files and directories deleted by delete or rename operation");
     public MetricsTimeVaryingInt numFileInfoOps =
                           new MetricsTimeVaryingInt("FileInfoOps", registry);
     public MetricsTimeVaryingInt numAddBlockOps = 
                           new MetricsTimeVaryingInt("AddBlockOps", registry);
 
-    public MetricsTimeVaryingRate transactions =
-                    new MetricsTimeVaryingRate("Transactions", registry, "Journal Transaction");
+    public MetricsTimeVaryingRate transactions = new MetricsTimeVaryingRate(
+      "Transactions", registry, "Journal Transaction");
     public MetricsTimeVaryingRate syncs =
                     new MetricsTimeVaryingRate("Syncs", registry, "Journal Sync");
-    public MetricsTimeVaryingInt transactionsBatchedInSync = 
-                    new MetricsTimeVaryingInt("JournalTransactionsBatchedInSync", registry, "Journal Transactions Batched In Sync");
+    public MetricsTimeVaryingInt transactionsBatchedInSync = new MetricsTimeVaryingInt(
+      "JournalTransactionsBatchedInSync", registry,
+      "Journal Transactions Batched In Sync");
     public MetricsTimeVaryingRate blockReport =
                     new MetricsTimeVaryingRate("blockReport", registry, "Block Report");
     public MetricsIntValue safeModeTime =

src/test/org/apache/hadoop/hdfs/server/namenode/metrics/TestNameNodeMetrics.java

@@ -32,7 +32,6 @@ import org.apache.hadoop.hdfs.DistributedFileSystem;
 import org.apache.hadoop.hdfs.MiniDFSCluster;
 import org.apache.hadoop.hdfs.protocol.LocatedBlock;
 import org.apache.hadoop.hdfs.server.namenode.FSNamesystem;
-import org.mortbay.log.Log;
 
 /**
  * Test for metrics published by the Namenode
@@ -55,7 +54,6 @@ public class TestNameNodeMetrics extends TestCase {
   private Random rand = new Random();
   private FSNamesystem namesystem;
   private NameNodeMetrics nnMetrics;
-  private NameNode nn;
 
   @Override
   protected void setUp() throws Exception {
@@ -64,8 +62,7 @@ public class TestNameNodeMetrics extends TestCase {
     namesystem = cluster.getNameNode().getNamesystem();
     fs = (DistributedFileSystem) cluster.getFileSystem();
     metrics = namesystem.getFSNamesystemMetrics();
-    nn = cluster.getNameNode();
-    nnMetrics = nn.getNameNodeMetrics();
+    nnMetrics = NameNode.getNameNodeMetrics();
   }
   
   @Override
@@ -84,20 +81,15 @@ public class TestNameNodeMetrics extends TestCase {
     // for some block related metrics to get updated)
     Thread.sleep(1000);
     metrics.doUpdates(null);
-  }
-
-  private void updateNNMetrics() throws Exception {
-    //Wait for nnmetrics update
-    Thread.sleep(1000);
     nnMetrics.doUpdates(null);
   }
- 
+
   private void readFile(FileSystem fileSys,String path) throws IOException {
     //Just read file so that getNumBlockLocations are incremented
     Path name = new Path(path);
     DataInputStream stm = fileSys.open(name);
     byte [] buffer = new byte[4];
-    int bytesRead =  stm.read(buffer,0,4);
+    stm.read(buffer,0,4);
     stm.close();
   }
 
@@ -111,6 +103,10 @@ public class TestNameNodeMetrics extends TestCase {
     int blockCapacity = namesystem.getBlockCapacity();
     updateMetrics();
     assertEquals(blockCapacity, metrics.blockCapacity.get());
+    
+    // File create operations is 1
+    // Number of files created is depth of <code>file</code> path
+    assertEquals(1, nnMetrics.numCreateFileOps.getPreviousIntervalValue());
 
     // Blocks are stored in a hashmap. Compute its capacity, which
     // doubles every time the number of entries reach the threshold.
@@ -123,6 +119,11 @@ public class TestNameNodeMetrics extends TestCase {
     assertEquals(blockCount, metrics.blocksTotal.get());
     assertEquals(blockCapacity, metrics.blockCapacity.get());
     fs.delete(new Path(file), true);
+    updateMetrics();
+
+    // Delete file operations and number of files deleted must be 1
+    assertEquals(1, nnMetrics.numDeleteFileOps.getPreviousIntervalValue());
+    assertEquals(1, nnMetrics.numFilesDeleted.getPreviousIntervalValue());
   }
   
   /** Corrupt a block and ensure metrics reflects it */
@@ -189,9 +190,6 @@ public class TestNameNodeMetrics extends TestCase {
    * @throws IOException in case of an error
    */
   public void testGetBlockLocationMetric() throws Exception{
-    final String METHOD_NAME = "TestGetBlockLocationMetric";
-    Log.info("Running test "+METHOD_NAME);
-  
     String file1_path = "/tmp/filePath";
 
     // When cluster starts first time there are no file  (read,create,open)
@@ -205,8 +203,7 @@ public class TestNameNodeMetrics extends TestCase {
 
     //Perform create file operation
     createFile(file1_path,100,(short)2);
-    // Update NameNode metrics
-    updateNNMetrics();
+    updateMetrics();
   
     //Create file does not change numGetBlockLocations metric
     //expect numGetBlockLocations = 0 for previous and current interval 
@@ -219,8 +216,7 @@ public class TestNameNodeMetrics extends TestCase {
     // Open and read file operation increments numGetBlockLocations
     // Perform read file operation on earlier created file
     readFile(fs, file1_path);
-    // Update NameNode metrics
-    updateNNMetrics();
+    updateMetrics();
     // Verify read file operation has incremented numGetBlockLocations by 1
     assertEquals("numGetBlockLocations for previous interval is incorrect",
     1,nnMetrics.numGetBlockLocations.getPreviousIntervalValue());
@@ -231,7 +227,7 @@ public class TestNameNodeMetrics extends TestCase {
     // opening and reading file  twice will increment numGetBlockLocations by 2
     readFile(fs, file1_path);
     readFile(fs, file1_path);
-    updateNNMetrics();
+    updateMetrics();
     assertEquals("numGetBlockLocations for previous interval is incorrect",
     2,nnMetrics.numGetBlockLocations.getPreviousIntervalValue());
     // Verify numGetBlockLocations for current interval is 0