Author: eli
Date: Fri Dec 10 07:02:27 2010
New Revision: 1044218
URL: http://svn.apache.org/viewvc?rev=1044218&view=rev
Log:
HDFS-1377. Quota bug for partial blocks allows quotas to be violated.
Modified:
hadoop/hdfs/trunk/CHANGES.txt
hadoop/hdfs/trunk/src/java/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java
hadoop/hdfs/trunk/src/java/org/apache/hadoop/hdfs/server/namenode/INodeDirectory.java
hadoop/hdfs/trunk/src/java/org/apache/hadoop/hdfs/server/namenode/INodeFile.java
hadoop/hdfs/trunk/src/test/hdfs/org/apache/hadoop/hdfs/TestQuota.java
Modified: hadoop/hdfs/trunk/CHANGES.txt
URL:
http://svn.apache.org/viewvc/hadoop/hdfs/trunk/CHANGES.txt?rev=1044218&r1=1044217&r2=1044218&view=diff
==============================================================================
--- hadoop/hdfs/trunk/CHANGES.txt (original)
+++ hadoop/hdfs/trunk/CHANGES.txt Fri Dec 10 07:02:27 2010
@@ -1474,3 +1474,5 @@ Release 0.20.1 - 2009-09-01
HDFS-761. Fix failure to process rename operation from edits log due to
quota verification. (suresh)
+
+ HDFS-1377. Quota bug for partial blocks allows quotas to be violated. (eli)
Modified:
hadoop/hdfs/trunk/src/java/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java
URL:
http://svn.apache.org/viewvc/hadoop/hdfs/trunk/src/java/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java?rev=1044218&r1=1044217&r2=1044218&view=diff
==============================================================================
---
hadoop/hdfs/trunk/src/java/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java
(original)
+++
hadoop/hdfs/trunk/src/java/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java
Fri Dec 10 07:02:27 2010
@@ -1095,21 +1095,10 @@ class FSDirectory implements Closeable {
/**
* Replaces the specified inode with the specified one.
*/
- void replaceNode(String path, INodeFile oldnode, INodeFile newnode)
- throws IOException, UnresolvedLinkException {
- replaceNode(path, oldnode, newnode, true);
- }
-
- /**
- * @see #replaceNode(String, INodeFile, INodeFile)
- */
- private void replaceNode(String path, INodeFile oldnode, INodeFile newnode,
- boolean updateDiskspace)
+ public void replaceNode(String path, INodeFile oldnode, INodeFile newnode)
throws IOException, UnresolvedLinkException {
writeLock();
try {
- long dsOld = oldnode.diskspaceConsumed();
-
//
// Remove the node from the namespace
//
@@ -1123,21 +1112,8 @@ class FSDirectory implements Closeable {
/* Currently oldnode and newnode are assumed to contain the same
* blocks. Otherwise, blocks need to be removed from the blocksMap.
*/
-
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 (BlockInfo b : newnode.getBlocks()) {
BlockInfo info = getBlockManager().addINode(b, newnode);
Modified:
hadoop/hdfs/trunk/src/java/org/apache/hadoop/hdfs/server/namenode/INodeDirectory.java
URL:
http://svn.apache.org/viewvc/hadoop/hdfs/trunk/src/java/org/apache/hadoop/hdfs/server/namenode/INodeDirectory.java?rev=1044218&r1=1044217&r2=1044218&view=diff
==============================================================================
---
hadoop/hdfs/trunk/src/java/org/apache/hadoop/hdfs/server/namenode/INodeDirectory.java
(original)
+++
hadoop/hdfs/trunk/src/java/org/apache/hadoop/hdfs/server/namenode/INodeDirectory.java
Fri Dec 10 07:02:27 2010
@@ -414,6 +414,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;
}
Modified:
hadoop/hdfs/trunk/src/java/org/apache/hadoop/hdfs/server/namenode/INodeFile.java
URL:
http://svn.apache.org/viewvc/hadoop/hdfs/trunk/src/java/org/apache/hadoop/hdfs/server/namenode/INodeFile.java?rev=1044218&r1=1044217&r2=1044218&view=diff
==============================================================================
---
hadoop/hdfs/trunk/src/java/org/apache/hadoop/hdfs/server/namenode/INodeFile.java
(original)
+++
hadoop/hdfs/trunk/src/java/org/apache/hadoop/hdfs/server/namenode/INodeFile.java
Fri Dec 10 07:02:27 2010
@@ -218,7 +218,7 @@ class INodeFile extends INode {
*/
if (blkArr.length > 0 && blkArr[blkArr.length-1] != null &&
isUnderConstruction()) {
- size += getPreferredBlockSize() - blocks[blocks.length-1].getNumBytes();
+ size += getPreferredBlockSize() - blkArr[blkArr.length-1].getNumBytes();
}
return size * getReplication();
}
Modified: hadoop/hdfs/trunk/src/test/hdfs/org/apache/hadoop/hdfs/TestQuota.java
URL:
http://svn.apache.org/viewvc/hadoop/hdfs/trunk/src/test/hdfs/org/apache/hadoop/hdfs/TestQuota.java?rev=1044218&r1=1044217&r2=1044218&view=diff
==============================================================================
--- hadoop/hdfs/trunk/src/test/hdfs/org/apache/hadoop/hdfs/TestQuota.java
(original)
+++ hadoop/hdfs/trunk/src/test/hdfs/org/apache/hadoop/hdfs/TestQuota.java Fri
Dec 10 07:02:27 2010
@@ -33,10 +33,11 @@ import org.apache.hadoop.security.UserGr
import org.apache.hadoop.hdfs.protocol.NSQuotaExceededException;
import org.apache.hadoop.hdfs.protocol.DSQuotaExceededException;
-import junit.framework.TestCase;
+import org.junit.Test;
+import static org.junit.Assert.*;
/** A class for testing quota-related commands */
-public class TestQuota extends TestCase {
+public class TestQuota {
private void runCommand(DFSAdmin admin, boolean expectError, String... args)
throws Exception {
@@ -56,6 +57,7 @@ public class TestQuota extends TestCase
/** Test quota related commands:
* setQuota, clrQuota, setSpaceQuota, clrSpaceQuota, and count
*/
+ @Test
public void testQuotaCommands() throws Exception {
final Configuration conf = new HdfsConfiguration();
// set a smaller block size so that we can test with smaller
@@ -281,6 +283,7 @@ public class TestQuota extends TestCase
/** Test commands that change the size of the name space:
* mkdirs, rename, and delete */
+ @Test
public void testNamespaceCommands() throws Exception {
final Configuration conf = new HdfsConfiguration();
final MiniDFSCluster cluster = new
MiniDFSCluster.Builder(conf).numDataNodes(2).build();
@@ -449,6 +452,7 @@ public class TestQuota extends TestCase
*
* This is based on testNamespaceCommands() above.
*/
+ @Test
public void testSpaceCommands() throws Exception {
final Configuration conf = new HdfsConfiguration();
// set a smaller block size so that we can test with smaller
@@ -641,4 +645,127 @@ 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.
+ */
+ @Test
+ public void testBlockAllocationAdjustsUsageConservatively()
+ throws Exception {
+ Configuration conf = new HdfsConfiguration();
+ final int BLOCK_SIZE = 6 * 1024;
+ conf.setInt(DFSConfigKeys.DFS_BLOCK_SIZE_KEY, BLOCK_SIZE);
+ MiniDFSCluster cluster =
+ new MiniDFSCluster.Builder(conf).numDataNodes(3).build();
+ 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 space 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 a 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.
+ */
+ @Test
+ public void testMultipleFilesSmallerThanOneBlock() throws Exception {
+ Configuration conf = new HdfsConfiguration();
+ final int BLOCK_SIZE = 6 * 1024;
+ conf.setInt(DFSConfigKeys.DFS_BLOCK_SIZE_KEY, BLOCK_SIZE);
+ MiniDFSCluster cluster =
+ new MiniDFSCluster.Builder(conf).numDataNodes(3).build();
+ 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();
+ }
+ }
}
