HDFS-1377. svn merge -c 1100336 from branch-0.20-security

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

CHANGES.txt

@@ -1,5 +1,9 @@
 Hadoop Change Log
 
+Release 0.20.205.0 - unreleased
+
+  BUG FIXES
+
 Release 0.20.204.0 - unreleased
 
   BUG FIXES
@@ -56,6 +60,8 @@ Release 0.20.204.0 - unreleased
     by throwing an error to indicate the editlog needs to be empty.
     (suresh)
 
+    HDFS-1377. Quota bug for partial blocks allows quotas to be violated. (eli)
+
   IMPROVEMENTS
 
     MAPREDUCE-2415. Distribute the user task logs on to multiple disks.

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

@@ -663,19 +663,9 @@ class FSDirectory implements FSConstants, Closeable {
   /**
    * Replaces the specified inode with the specified one.
    */
-  void replaceNode(String path, INodeFile oldnode, INodeFile newnode) 
-                                                   throws IOException {
-    replaceNode(path, oldnode, newnode, true);
-  }
-  
-  /**
-   * @see #replaceNode(String, INodeFile, INodeFile)
-   */
-  private void replaceNode(String path, INodeFile oldnode, INodeFile newnode,
-                           boolean updateDiskspace) throws IOException {    
+  void replaceNode(String path, INodeFile oldnode, INodeFile newnode)
+      throws IOException {
     synchronized (rootDir) {
-      long dsOld = oldnode.diskspaceConsumed();
-      
       //
       // Remove the node from the namespace 
       //
@@ -692,18 +682,6 @@ class FSDirectory implements FSConstants, Closeable {
       
       rootDir.addNode(path, newnode); 
 
-      //check if disk space needs to be updated.
-      long dsNew = 0;
-      if (updateDiskspace && (dsNew = newnode.diskspaceConsumed()) != dsOld) {
-        try {
-          updateSpaceConsumed(path, 0, dsNew-dsOld);
-        } catch (QuotaExceededException e) {
-          // undo
-          replaceNode(path, newnode, oldnode, false);
-          throw e;
-        }
-      }
-      
       int index = 0;
       for (Block b : newnode.getBlocks()) {
         BlockInfo info = namesystem.blocksMap.addINode(b, newnode);

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

@@ -338,6 +338,15 @@ class INodeDirectory extends INode {
         child.computeContentSummary(summary);
       }
     }
+    if (this instanceof INodeDirectoryWithQuota) {
+      // Warn if the cached and computed diskspace values differ
+      INodeDirectoryWithQuota node = (INodeDirectoryWithQuota)this;
+      long space = node.diskspaceConsumed();
+      if (-1 != node.getDsQuota() && space != summary[3]) {
+        NameNode.LOG.warn("Inconsistent diskspace for directory "
+          +getLocalName()+". Cached: "+space+" Computed: "+summary[3]);
+      }
+    }
     summary[2]++;
     return summary;
   }

src/test/org/apache/hadoop/hdfs/TestQuota.java

@@ -695,4 +695,124 @@ public class TestQuota extends TestCase {
       cluster.shutdown();
     }
   }
+
+  /**
+   * Violate a space quota using files of size < 1 block. Test that
+   * block allocation conservatively assumes that for quota checking
+   * the entire space of the block is used.
+   */
+  public void testBlockAllocationAdjustUsageConservatively() throws Exception {
+    Configuration conf = new Configuration();
+    final int BLOCK_SIZE = 6 * 1024;
+    conf.set("dfs.block.size", Integer.toString(BLOCK_SIZE));
+    MiniDFSCluster cluster = new MiniDFSCluster(conf, 3, true, null);
+    cluster.waitActive();
+    FileSystem fs = cluster.getFileSystem();
+    DFSAdmin admin = new DFSAdmin(conf);
+
+    try {
+      Path dir = new Path("/test");
+      Path file1 = new Path("/test/test1");
+      Path file2 = new Path("/test/test2");
+      boolean exceededQuota = false;
+      final int QUOTA_SIZE = 3 * BLOCK_SIZE; // total usage including repl.
+      final int FILE_SIZE = BLOCK_SIZE / 2; 
+      ContentSummary c;
+
+      // Create the directory and set the quota
+      assertTrue(fs.mkdirs(dir));
+      runCommand(admin, false, "-setSpaceQuota", Integer.toString(QUOTA_SIZE),
+		 dir.toString());
+    
+      // Creating one file should use half the quota
+      DFSTestUtil.createFile(fs, file1, FILE_SIZE, (short)3, 1L);
+      DFSTestUtil.waitReplication(fs, file1, (short)3);
+      c = fs.getContentSummary(dir);
+      assertEquals("Quota is half consumed", QUOTA_SIZE / 2,
+		   c.getSpaceConsumed());
+
+      // We can not create the 2nd file because even though the total
+      // spaced used by two files (2 * 3 * 512/2) would fit within the
+      // quota (3 * 512) when a block for a file is created the space
+      // used is adjusted conservatively (3 * block size, ie assumes a
+      // full block is written) which will violate the quota (3 *
+      // block size) since we've already used half the quota for the
+      // first file.
+      try {
+	DFSTestUtil.createFile(fs, file2, FILE_SIZE, (short)3, 1L);
+      } catch (QuotaExceededException e) {
+	exceededQuota = true;
+      }
+      assertTrue("Quota not exceeded", exceededQuota);
+    } finally {
+      cluster.shutdown();
+    }
+  }
+
+  /**
+   * Like the previous test but create many files. This covers bugs
+   * where the quota adjustment is incorrect but it takes many files
+   * to accrue a big enough accounting error to violate the quota.
+   */
+  public void testMultipleFilesSmallerThanOneBlock() throws Exception {
+    Configuration conf = new Configuration();
+    final int BLOCK_SIZE = 6 * 1024;
+    conf.set("dfs.block.size", Integer.toString(BLOCK_SIZE));
+    MiniDFSCluster cluster = new MiniDFSCluster(conf, 3, true, null);
+    cluster.waitActive();
+    FileSystem fs = cluster.getFileSystem();
+    DFSAdmin admin = new DFSAdmin(conf);
+
+    try {
+      Path dir = new Path("/test");
+      boolean exceededQuota = false;
+      ContentSummary c;
+      //   1kb file
+      //   6kb block
+      // 192kb quota
+      final int FILE_SIZE = 1024; 
+      final int QUOTA_SIZE = 32 * (int)fs.getDefaultBlockSize();
+      assertEquals(6 * 1024, fs.getDefaultBlockSize());
+      assertEquals(192 * 1024, QUOTA_SIZE);
+
+      // Create the dir and set the quota. We need to enable the quota before
+      // writing the files as setting the quota afterwards will over-write 
+      // the cached disk space used for quota verification with the actual
+      // amount used as calculated by INode#spaceConsumedInTree.
+      assertTrue(fs.mkdirs(dir));
+      runCommand(admin, false, "-setSpaceQuota", Integer.toString(QUOTA_SIZE),
+		 dir.toString());
+
+      // We can create at most 59 files because block allocation is
+      // conservative and initially assumes a full block is used, so we
+      // need to leave at least 3 * BLOCK_SIZE free space when allocating 
+      // the last block: (58 * 3 * 1024) + (3 * 6 * 1024) = 192kb
+      for (int i = 0; i < 59; i++) {
+	Path file = new Path("/test/test" + i);
+	DFSTestUtil.createFile(fs, file, FILE_SIZE, (short)3, 1L);
+	DFSTestUtil.waitReplication(fs, file, (short)3);
+      }
+
+      // Should account for all 59 files (almost QUOTA_SIZE)
+      c = fs.getContentSummary(dir);
+      assertEquals("Invalid space consumed", 
+		   59 * FILE_SIZE * 3, 
+		   c.getSpaceConsumed());
+      assertEquals("Invalid space consumed",
+		   QUOTA_SIZE - (59 * FILE_SIZE * 3),
+		   3 * (fs.getDefaultBlockSize() - FILE_SIZE));
+
+      // Now check that trying to create another file violates the quota
+      try {
+	Path file = new Path("/test/test59");
+	DFSTestUtil.createFile(fs, file, FILE_SIZE, (short) 3, 1L);
+	DFSTestUtil.waitReplication(fs, file, (short) 3);
+      } catch (QuotaExceededException e) {
+	exceededQuota = true;
+      }
+      assertTrue("Quota not exceeded", exceededQuota);
+    } finally {
+      cluster.shutdown();
+    }
+  }
 }