You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-commits@hadoop.apache.org by vi...@apache.org on 2015/09/02 01:54:35 UTC
hadoop git commit: HDFS-7587. Edit log corruption can happen if
append fails with a quota violation. Contributed by Jing Zhao.
Repository: hadoop
Updated Branches:
refs/heads/branch-2.6.1 a9bb641d5 -> 49ef26b5a
HDFS-7587. Edit log corruption can happen if append fails with a quota violation. Contributed by Jing Zhao.
Committed Ming Ma's 2.6 patch.
(cherry picked from commit 7f0bb5d3fe0db2e6b9354c8d8a1b603f2390184f)
Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo
Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/49ef26b5
Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/49ef26b5
Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/49ef26b5
Branch: refs/heads/branch-2.6.1
Commit: 49ef26b5ac299d0f17afce4e6e7e30afdfef4d18
Parents: a9bb641
Author: Jing Zhao <ji...@apache.org>
Authored: Wed Mar 18 18:51:14 2015 -0700
Committer: Vinod Kumar Vavilapalli <vi...@apache.org>
Committed: Tue Sep 1 16:33:30 2015 -0700
----------------------------------------------------------------------
hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt | 3 +
.../hdfs/server/namenode/FSDirectory.java | 8 +-
.../hdfs/server/namenode/FSEditLogLoader.java | 2 +-
.../hdfs/server/namenode/FSNamesystem.java | 86 +++++++++++++++-----
.../hdfs/server/namenode/INodesInPath.java | 4 +
.../namenode/TestDiskspaceQuotaUpdate.java | 42 ++++++++++
6 files changed, 122 insertions(+), 23 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/hadoop/blob/49ef26b5/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
----------------------------------------------------------------------
diff --git a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
index ed845f5..44a7139 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
+++ b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
@@ -94,6 +94,9 @@ Release 2.6.1 - UNRELEASED
HDFS-7830. DataNode does not release the volume lock when adding a volume
fails. (Lei Xu via Colin P. Mccabe)
+ HDFS-7587. Edit log corruption can happen if append fails with a quota
+ violation. (jing9)
+
Release 2.6.0 - 2014-11-18
INCOMPATIBLE CHANGES
http://git-wip-us.apache.org/repos/asf/hadoop/blob/49ef26b5/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java
----------------------------------------------------------------------
diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java
index 9ca50c4..95877ab 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java
+++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java
@@ -267,6 +267,10 @@ public class FSDirectory implements Closeable {
}
}
+ boolean shouldSkipQuotaChecks() {
+ return skipQuotaCheck;
+ }
+
/** Enable quota verification */
void enableQuotaChecks() {
skipQuotaCheck = false;
@@ -1738,7 +1742,7 @@ public class FSDirectory implements Closeable {
* update quota of each inode and check to see if quota is exceeded.
* See {@link #updateCount(INodesInPath, long, long, boolean)}
*/
- private void updateCountNoQuotaCheck(INodesInPath inodesInPath,
+ void updateCountNoQuotaCheck(INodesInPath inodesInPath,
int numOfINodes, long nsDelta, long dsDelta) {
assert hasWriteLock();
try {
@@ -1877,7 +1881,7 @@ public class FSDirectory implements Closeable {
* Pass null if a node is not being moved.
* @throws QuotaExceededException if quota limit is exceeded.
*/
- private static void verifyQuota(INode[] inodes, int pos, long nsDelta,
+ static void verifyQuota(INode[] inodes, int pos, long nsDelta,
long dsDelta, INode commonAncestor) throws QuotaExceededException {
if (nsDelta <= 0 && dsDelta <= 0) {
// if quota is being freed or not being consumed
http://git-wip-us.apache.org/repos/asf/hadoop/blob/49ef26b5/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogLoader.java
----------------------------------------------------------------------
diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogLoader.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogLoader.java
index 7dfe688..cb5afbb 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogLoader.java
+++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogLoader.java
@@ -387,7 +387,7 @@ public class FSEditLogLoader {
"for append");
}
LocatedBlock lb = fsNamesys.prepareFileForWrite(path,
- oldFile, addCloseOp.clientName, addCloseOp.clientMachine, false, iip.getLatestSnapshotId(), false);
+ iip, addCloseOp.clientName, addCloseOp.clientMachine, false, false);
newFile = INodeFile.valueOf(fsDir.getINode(path),
path, true);
http://git-wip-us.apache.org/repos/asf/hadoop/blob/49ef26b5/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
----------------------------------------------------------------------
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 5541637..c92b431 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
@@ -2872,8 +2872,8 @@ public class FSNamesystem implements Namesystem, FSClusterStats,
throw new IOException("append: lastBlock=" + lastBlock +
" of src=" + src + " is not sufficiently replicated yet.");
}
- return prepareFileForWrite(src, myFile, holder, clientMachine, true,
- iip.getLatestSnapshotId(), logRetryCache);
+ return prepareFileForWrite(src, iip, holder, clientMachine, true,
+ logRetryCache);
} catch (IOException ie) {
NameNode.stateChangeLog.warn("DIR* NameSystem.append: " +ie.getMessage());
throw ie;
@@ -2895,31 +2895,77 @@ public class FSNamesystem implements Namesystem, FSClusterStats,
* @throws UnresolvedLinkException
* @throws IOException
*/
- LocatedBlock prepareFileForWrite(String src, INodeFile file,
- String leaseHolder, String clientMachine,
- boolean writeToEditLog,
- int latestSnapshot, boolean logRetryCache)
- throws IOException {
- file.recordModification(latestSnapshot);
- final INodeFile cons = file.toUnderConstruction(leaseHolder, clientMachine);
+ LocatedBlock prepareFileForWrite(String src, INodesInPath iip,
+ String leaseHolder, String clientMachine, boolean writeToEditLog,
+ boolean logRetryCache) throws IOException {
+ final INodeFile file = iip.getLastINode().asFile();
+ final Quota.Counts delta = verifyQuotaForUCBlock(file, iip);
- leaseManager.addLease(cons.getFileUnderConstructionFeature()
- .getClientName(), src);
-
- LocatedBlock ret = blockManager.convertLastBlockToUnderConstruction(cons);
- if (ret != null) {
- // update the quota: use the preferred block size for UC block
- final long diff = file.getPreferredBlockSize() - ret.getBlockSize();
- dir.updateSpaceConsumed(src, 0, diff * file.getBlockReplication());
+ file.recordModification(iip.getLatestSnapshotId());
+ file.toUnderConstruction(leaseHolder, clientMachine);
+
+ leaseManager.addLease(
+ file.getFileUnderConstructionFeature().getClientName(), src);
+
+ LocatedBlock ret = blockManager.convertLastBlockToUnderConstruction(file);
+ if (ret != null && delta != null) {
+ Preconditions.checkState(delta.get(Quota.DISKSPACE) >= 0,
+ "appending to a block with size larger than the preferred block size");
+ dir.writeLock();
+ try {
+ dir.updateCountNoQuotaCheck(iip, iip.length() - 1,
+ delta.get(Quota.NAMESPACE), delta.get(Quota.DISKSPACE));
+ } finally {
+ dir.writeUnlock();
+ }
}
if (writeToEditLog) {
- getEditLog().logOpenFile(src, cons, false, logRetryCache);
+ getEditLog().logOpenFile(src, file, false, logRetryCache);
}
return ret;
}
/**
+ * Verify quota when using the preferred block size for UC block. This is
+ * usually used by append and truncate
+ * @throws QuotaExceededException when violating the storage quota
+ * @return expected quota usage update. null means no change or no need to
+ * update quota usage later
+ */
+ private Quota.Counts verifyQuotaForUCBlock(INodeFile file, INodesInPath iip)
+ throws QuotaExceededException {
+ if (!isImageLoaded() || dir.shouldSkipQuotaChecks()) {
+ // Do not check quota if editlog is still being processed
+ return null;
+ }
+ if (file.getLastBlock() != null) {
+ final Quota.Counts delta = computeQuotaDeltaForUCBlock(file);
+ dir.readLock();
+ try {
+ FSDirectory.verifyQuota(iip.getINodes(), iip.length() - 1,
+ delta.get(Quota.NAMESPACE), delta.get(Quota.DISKSPACE), null);
+ return delta;
+ } finally {
+ dir.readUnlock();
+ }
+ }
+ return null;
+ }
+
+ /** Compute quota change for converting a complete block to a UC block */
+ private Quota.Counts computeQuotaDeltaForUCBlock(INodeFile file) {
+ final BlockInfo lastBlock = file.getLastBlock();
+ if (lastBlock != null) {
+ final long diff = file.getPreferredBlockSize() - lastBlock.getNumBytes();
+ final short repl = file.getBlockReplication();
+ return Quota.Counts.newInstance(0, diff * repl);
+ } else {
+ return Quota.Counts.newInstance();
+ }
+ }
+
+ /**
* Recover lease;
* Immediately revoke the lease of the current lease holder and start lease
* recovery so that the file can be forced to be closed.
@@ -3318,7 +3364,7 @@ public class FSNamesystem implements Namesystem, FSClusterStats,
// doesn't match up with what we think is the last block. There are
// four possibilities:
// 1) This is the first block allocation of an append() pipeline
- // which started appending exactly at a block boundary.
+ // which started appending exactly at or exceeding the block boundary.
// In this case, the client isn't passed the previous block,
// so it makes the allocateBlock() call with previous=null.
// We can distinguish this since the last block of the file
@@ -3343,7 +3389,7 @@ public class FSNamesystem implements Namesystem, FSClusterStats,
BlockInfo penultimateBlock = pendingFile.getPenultimateBlock();
if (previous == null &&
lastBlockInFile != null &&
- lastBlockInFile.getNumBytes() == pendingFile.getPreferredBlockSize() &&
+ lastBlockInFile.getNumBytes() >= pendingFile.getPreferredBlockSize() &&
lastBlockInFile.isComplete()) {
// Case 1
if (NameNode.stateChangeLog.isDebugEnabled()) {
http://git-wip-us.apache.org/repos/asf/hadoop/blob/49ef26b5/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodesInPath.java
----------------------------------------------------------------------
diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodesInPath.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodesInPath.java
index c74ebb0..7e45a1d 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodesInPath.java
+++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodesInPath.java
@@ -67,6 +67,10 @@ public class INodesInPath {
return iip;
}
+ public int length() {
+ return inodes.length;
+ }
+
/**
* Given some components, create a path name.
* @param components The path components
http://git-wip-us.apache.org/repos/asf/hadoop/blob/49ef26b5/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestDiskspaceQuotaUpdate.java
----------------------------------------------------------------------
diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestDiskspaceQuotaUpdate.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestDiskspaceQuotaUpdate.java
index fad8807..f4e1a8b 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestDiskspaceQuotaUpdate.java
+++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestDiskspaceQuotaUpdate.java
@@ -32,7 +32,9 @@ import org.apache.hadoop.hdfs.DFSTestUtil;
import org.apache.hadoop.hdfs.DistributedFileSystem;
import org.apache.hadoop.hdfs.MiniDFSCluster;
import org.apache.hadoop.hdfs.client.HdfsDataOutputStream;
+import org.apache.hadoop.hdfs.protocol.DSQuotaExceededException;
import org.junit.After;
+import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
@@ -182,4 +184,44 @@ public class TestDiskspaceQuotaUpdate {
assertEquals(2, ns); // foo and bar
assertEquals((BLOCKSIZE * 2 + BLOCKSIZE / 2) * REPLICATION, ds);
}
+
+ /**
+ * Test append over storage quota does not mark file as UC or create lease
+ */
+ @Test (timeout=60000)
+ public void testAppendOverStorageQuota() throws Exception {
+ final Path dir = new Path("/TestAppendOverQuota");
+ final Path file = new Path(dir, "file");
+
+ // create partial block file
+ dfs.mkdirs(dir);
+ DFSTestUtil.createFile(dfs, file, BLOCKSIZE/2, REPLICATION, seed);
+
+ // lower quota to cause exception when appending to partial block
+ dfs.setQuota(dir, Long.MAX_VALUE - 1, 1);
+ final INodeDirectory dirNode = fsdir.getINode4Write(dir.toString())
+ .asDirectory();
+ final long spaceUsed = dirNode.getDirectoryWithQuotaFeature()
+ .getSpaceConsumed().get(Quota.DISKSPACE);
+ try {
+ DFSTestUtil.appendFile(dfs, file, BLOCKSIZE);
+ Assert.fail("append didn't fail");
+ } catch (DSQuotaExceededException e) {
+ // ignore
+ }
+
+ LeaseManager lm = cluster.getNamesystem().getLeaseManager();
+ // check that the file exists, isn't UC, and has no dangling lease
+ INodeFile inode = fsdir.getINode(file.toString()).asFile();
+ Assert.assertNotNull(inode);
+ Assert.assertFalse("should not be UC", inode.isUnderConstruction());
+ Assert.assertNull("should not have a lease", lm.getLeaseByPath(file.toString()));
+ // make sure the quota usage is unchanged
+ final long newSpaceUsed = dirNode.getDirectoryWithQuotaFeature()
+ .getSpaceConsumed().get(Quota.DISKSPACE);
+ assertEquals(spaceUsed, newSpaceUsed);
+ // make sure edits aren't corrupted
+ dfs.recoverLease(file);
+ cluster.restartNameNodes();
+ }
}