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 wh...@apache.org on 2014/12/11 08:01:33 UTC
hadoop git commit: HDFS-7463. Simplify
FSNamesystem#getBlockLocationsUpdateTimes. Contributed by Haohui Mai.
Repository: hadoop
Updated Branches:
refs/heads/branch-2 6e06a51e3 -> 9e1c05981
HDFS-7463. Simplify FSNamesystem#getBlockLocationsUpdateTimes. Contributed by Haohui Mai.
Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo
Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/9e1c0598
Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/9e1c0598
Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/9e1c0598
Branch: refs/heads/branch-2
Commit: 9e1c05981506dade56654dbf18f9d63cb6629532
Parents: 6e06a51
Author: Haohui Mai <wh...@apache.org>
Authored: Wed Dec 10 23:01:17 2014 -0800
Committer: Haohui Mai <wh...@apache.org>
Committed: Wed Dec 10 23:01:28 2014 -0800
----------------------------------------------------------------------
hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt | 2 +
.../hdfs/server/namenode/FSNamesystem.java | 218 +++++++++----------
.../hdfs/server/namenode/NamenodeFsck.java | 9 +-
.../org/apache/hadoop/hdfs/TestGetBlocks.java | 4 +-
.../hdfs/server/namenode/NameNodeAdapter.java | 4 +-
.../hadoop/hdfs/server/namenode/TestFsck.java | 6 +-
6 files changed, 118 insertions(+), 125 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/hadoop/blob/9e1c0598/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 68271f7..2f7ac12 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
+++ b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
@@ -192,6 +192,8 @@ Release 2.7.0 - UNRELEASED
HDFS-7498. Simplify the logic in INodesInPath. (jing9)
+ HDFS-7463. Simplify FSNamesystem#getBlockLocationsUpdateTimes. (wheat9)
+
OPTIMIZATIONS
HDFS-7454. Reduce memory footprint for AclEntries in NameNode.
http://git-wip-us.apache.org/repos/asf/hadoop/blob/9e1c0598/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 669b13c..7f43722 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
@@ -1739,27 +1739,76 @@ public class FSNamesystem implements Namesystem, FSNamesystemMBean,
logAuditEvent(true, "setOwner", srcArg, null, resultingStat);
}
+ static class GetBlockLocationsResult {
+ final INodesInPath iip;
+ final LocatedBlocks blocks;
+ boolean updateAccessTime() {
+ return iip != null;
+ }
+ private GetBlockLocationsResult(INodesInPath iip, LocatedBlocks blocks) {
+ this.iip = iip;
+ this.blocks = blocks;
+ }
+ }
+
/**
* Get block locations within the specified range.
* @see ClientProtocol#getBlockLocations(String, long, long)
*/
LocatedBlocks getBlockLocations(String clientMachine, String src,
- long offset, long length) throws AccessControlException,
- FileNotFoundException, UnresolvedLinkException, IOException {
- LocatedBlocks blocks = getBlockLocations(src, offset, length, true, true,
- true);
+ long offset, long length) throws IOException {
+ checkOperation(OperationCategory.READ);
+ GetBlockLocationsResult res = null;
+ readLock();
+ try {
+ checkOperation(OperationCategory.READ);
+ res = getBlockLocations(src, offset, length, true, true);
+ } catch (AccessControlException e) {
+ logAuditEvent(false, "open", src);
+ throw e;
+ } finally {
+ readUnlock();
+ }
+
+ logAuditEvent(true, "open", src);
+
+ if (res == null) {
+ return null;
+ }
+
+ if (res.updateAccessTime()) {
+ writeLock();
+ final long now = now();
+ try {
+ checkOperation(OperationCategory.WRITE);
+ INode inode = res.iip.getLastINode();
+ boolean updateAccessTime = now > inode.getAccessTime() +
+ getAccessTimePrecision();
+ if (!isInSafeMode() && updateAccessTime) {
+ boolean changed = dir.setTimes(
+ inode, -1, now, false, res.iip.getLatestSnapshotId());
+ if (changed) {
+ getEditLog().logTimes(src, -1, now);
+ }
+ }
+ } catch (Throwable e) {
+ LOG.warn("Failed to update the access time of " + src, e);
+ } finally {
+ writeUnlock();
+ }
+ }
+
+ LocatedBlocks blocks = res.blocks;
if (blocks != null) {
- blockManager.getDatanodeManager().sortLocatedBlocks(clientMachine,
- blocks.getLocatedBlocks());
+ blockManager.getDatanodeManager().sortLocatedBlocks(
+ clientMachine, blocks.getLocatedBlocks());
// lastBlock is not part of getLocatedBlocks(), might need to sort it too
LocatedBlock lastBlock = blocks.getLastLocatedBlock();
if (lastBlock != null) {
- ArrayList<LocatedBlock> lastBlockList =
- Lists.newArrayListWithCapacity(1);
- lastBlockList.add(lastBlock);
- blockManager.getDatanodeManager().sortLocatedBlocks(clientMachine,
- lastBlockList);
+ ArrayList<LocatedBlock> lastBlockList = Lists.newArrayList(lastBlock);
+ blockManager.getDatanodeManager().sortLocatedBlocks(
+ clientMachine, lastBlockList);
}
}
return blocks;
@@ -1768,24 +1817,11 @@ public class FSNamesystem implements Namesystem, FSNamesystemMBean,
/**
* Get block locations within the specified range.
* @see ClientProtocol#getBlockLocations(String, long, long)
- * @throws FileNotFoundException, UnresolvedLinkException, IOException
+ * @throws IOException
*/
- LocatedBlocks getBlockLocations(String src, long offset, long length,
- boolean doAccessTime, boolean needBlockToken, boolean checkSafeMode)
- throws FileNotFoundException, UnresolvedLinkException, IOException {
- try {
- return getBlockLocationsInt(src, offset, length, doAccessTime,
- needBlockToken, checkSafeMode);
- } catch (AccessControlException e) {
- logAuditEvent(false, "open", src);
- throw e;
- }
- }
-
- private LocatedBlocks getBlockLocationsInt(String src, long offset,
- long length, boolean doAccessTime, boolean needBlockToken,
- boolean checkSafeMode)
- throws FileNotFoundException, UnresolvedLinkException, IOException {
+ GetBlockLocationsResult getBlockLocations(
+ String src, long offset, long length, boolean needBlockToken,
+ boolean checkSafeMode) throws IOException {
if (offset < 0) {
throw new HadoopIllegalArgumentException(
"Negative offset is not supported. File: " + src);
@@ -1794,16 +1830,16 @@ public class FSNamesystem implements Namesystem, FSNamesystemMBean,
throw new HadoopIllegalArgumentException(
"Negative length is not supported. File: " + src);
}
- final LocatedBlocks ret = getBlockLocationsUpdateTimes(src,
- offset, length, doAccessTime, needBlockToken);
- logAuditEvent(true, "open", src);
+ final GetBlockLocationsResult ret = getBlockLocationsInt(
+ src, offset, length, needBlockToken);
+
if (checkSafeMode && isInSafeMode()) {
- for (LocatedBlock b : ret.getLocatedBlocks()) {
+ for (LocatedBlock b : ret.blocks.getLocatedBlocks()) {
// if safemode & no block locations yet then throw safemodeException
if ((b.getLocations() == null) || (b.getLocations().length == 0)) {
SafeModeException se = new SafeModeException(
"Zero blocklocations for " + src, safeMode);
- if (haEnabled && haContext != null &&
+ if (haEnabled && haContext != null &&
haContext.getState().getServiceState() == HAServiceState.ACTIVE) {
throw new RetriableException(se);
} else {
@@ -1815,95 +1851,49 @@ public class FSNamesystem implements Namesystem, FSNamesystemMBean,
return ret;
}
- /*
- * Get block locations within the specified range, updating the
- * access times if necessary.
- */
- private LocatedBlocks getBlockLocationsUpdateTimes(final String srcArg,
- long offset, long length, boolean doAccessTime, boolean needBlockToken)
+ private GetBlockLocationsResult getBlockLocationsInt(
+ final String srcArg, long offset, long length, boolean needBlockToken)
throws IOException {
String src = srcArg;
FSPermissionChecker pc = getPermissionChecker();
byte[][] pathComponents = FSDirectory.getPathComponentsForReservedPath(src);
- for (int attempt = 0; attempt < 2; attempt++) {
- boolean isReadOp = (attempt == 0);
- if (isReadOp) { // first attempt is with readlock
- checkOperation(OperationCategory.READ);
- readLock();
- } else { // second attempt is with write lock
- checkOperation(OperationCategory.WRITE);
- writeLock(); // writelock is needed to set accesstime
- }
- try {
- if (isReadOp) {
- checkOperation(OperationCategory.READ);
- } else {
- checkOperation(OperationCategory.WRITE);
- }
- src = dir.resolvePath(pc, src, pathComponents);
- final INodesInPath iip = dir.getINodesInPath(src, true);
- if (isPermissionEnabled) {
- dir.checkPathAccess(pc, iip, FsAction.READ);
- }
+ src = dir.resolvePath(pc, src, pathComponents);
+ final INodesInPath iip = dir.getINodesInPath(src, true);
+ final INodeFile inode = INodeFile.valueOf(iip.getLastINode(), src);
+ if (isPermissionEnabled) {
+ dir.checkPathAccess(pc, iip, FsAction.READ);
+ checkUnreadableBySuperuser(pc, inode, iip.getPathSnapshotId());
+ }
- // if the namenode is in safemode, then do not update access time
- if (isInSafeMode()) {
- doAccessTime = false;
- }
+ final long fileSize = iip.isSnapshot()
+ ? inode.computeFileSize(iip.getPathSnapshotId())
+ : inode.computeFileSizeNotIncludingLastUcBlock();
+ boolean isUc = inode.isUnderConstruction();
+ if (iip.isSnapshot()) {
+ // if src indicates a snapshot file, we need to make sure the returned
+ // blocks do not exceed the size of the snapshot file.
+ length = Math.min(length, fileSize - offset);
+ isUc = false;
+ }
- final INodeFile inode = INodeFile.valueOf(iip.getLastINode(), src);
- if (isPermissionEnabled) {
- checkUnreadableBySuperuser(pc, inode, iip.getPathSnapshotId());
- }
- if (!iip.isSnapshot() //snapshots are readonly, so don't update atime.
- && doAccessTime && isAccessTimeSupported()) {
- final long now = now();
- if (now > inode.getAccessTime() + getAccessTimePrecision()) {
- // if we have to set access time but we only have the readlock, then
- // restart this entire operation with the writeLock.
- if (isReadOp) {
- continue;
- }
- boolean changed = dir.setTimes(inode, -1, now, false,
- iip.getLatestSnapshotId());
- if (changed) {
- getEditLog().logTimes(src, -1, now);
- }
- }
- }
- final long fileSize = iip.isSnapshot() ?
- inode.computeFileSize(iip.getPathSnapshotId())
- : inode.computeFileSizeNotIncludingLastUcBlock();
- boolean isUc = inode.isUnderConstruction();
- if (iip.isSnapshot()) {
- // if src indicates a snapshot file, we need to make sure the returned
- // blocks do not exceed the size of the snapshot file.
- length = Math.min(length, fileSize - offset);
- isUc = false;
- }
+ final FileEncryptionInfo feInfo =
+ FSDirectory.isReservedRawName(srcArg) ? null
+ : dir.getFileEncryptionInfo(inode, iip.getPathSnapshotId(), iip);
- final FileEncryptionInfo feInfo =
- FSDirectory.isReservedRawName(srcArg) ?
- null : dir.getFileEncryptionInfo(inode, iip.getPathSnapshotId(),
- iip);
-
- final LocatedBlocks blocks =
- blockManager.createLocatedBlocks(inode.getBlocks(), fileSize,
- isUc, offset, length, needBlockToken, iip.isSnapshot(), feInfo);
- // Set caching information for the located blocks.
- for (LocatedBlock lb: blocks.getLocatedBlocks()) {
- cacheManager.setCachedLocations(lb);
- }
- return blocks;
- } finally {
- if (isReadOp) {
- readUnlock();
- } else {
- writeUnlock();
- }
- }
+ final LocatedBlocks blocks = blockManager.createLocatedBlocks(
+ inode.getBlocks(), fileSize, isUc, offset, length, needBlockToken,
+ iip.isSnapshot(), feInfo);
+
+ // Set caching information for the located blocks.
+ for (LocatedBlock lb : blocks.getLocatedBlocks()) {
+ cacheManager.setCachedLocations(lb);
}
- return null; // can never reach here
+
+ final long now = now();
+ boolean updateAccessTime = isAccessTimeSupported() && !isInSafeMode()
+ && !iip.isSnapshot()
+ && now > inode.getAccessTime() + getAccessTimePrecision();
+ return new GetBlockLocationsResult(updateAccessTime ? iip : null, blocks);
}
/**
http://git-wip-us.apache.org/repos/asf/hadoop/blob/9e1c0598/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NamenodeFsck.java
----------------------------------------------------------------------
diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NamenodeFsck.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NamenodeFsck.java
index 2413401..e0c1301 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NamenodeFsck.java
+++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/NamenodeFsck.java
@@ -441,12 +441,15 @@ public class NamenodeFsck implements DataEncryptionKeyFactory {
long fileLen = file.getLen();
// Get block locations without updating the file access time
// and without block access tokens
- LocatedBlocks blocks;
+ LocatedBlocks blocks = null;
+ FSNamesystem fsn = namenode.getNamesystem();
+ fsn.readLock();
try {
- blocks = namenode.getNamesystem().getBlockLocations(path, 0,
- fileLen, false, false, false);
+ blocks = fsn.getBlockLocations(path, 0, fileLen, false, false).blocks;
} catch (FileNotFoundException fnfe) {
blocks = null;
+ } finally {
+ fsn.readUnlock();
}
if (blocks == null) { // the file is deleted
return;
http://git-wip-us.apache.org/repos/asf/hadoop/blob/9e1c0598/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestGetBlocks.java
----------------------------------------------------------------------
diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestGetBlocks.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestGetBlocks.java
index d64c50c..d331de9 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestGetBlocks.java
+++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestGetBlocks.java
@@ -169,9 +169,7 @@ public class TestGetBlocks {
if (stm != null) {
stm.close();
}
- if (client != null) {
- client.close();
- }
+ client.close();
cluster.shutdown();
}
}
http://git-wip-us.apache.org/repos/asf/hadoop/blob/9e1c0598/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/NameNodeAdapter.java
----------------------------------------------------------------------
diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/NameNodeAdapter.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/NameNodeAdapter.java
index 61e7f14..7aad378 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/NameNodeAdapter.java
+++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/NameNodeAdapter.java
@@ -64,8 +64,8 @@ public class NameNodeAdapter {
*/
public static LocatedBlocks getBlockLocations(NameNode namenode,
String src, long offset, long length) throws IOException {
- return namenode.getNamesystem().getBlockLocations(
- src, offset, length, false, true, true);
+ return namenode.getNamesystem().getBlockLocations("foo",
+ src, offset, length);
}
public static HdfsFileStatus getFileInfo(NameNode namenode, String src,
http://git-wip-us.apache.org/repos/asf/hadoop/blob/9e1c0598/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFsck.java
----------------------------------------------------------------------
diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFsck.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFsck.java
index f53126b..5110cd1 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFsck.java
+++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFsck.java
@@ -996,9 +996,9 @@ public class TestFsck {
DatanodeManager dnManager = mock(DatanodeManager.class);
when(namenode.getNamesystem()).thenReturn(fsName);
- when(fsName.getBlockLocations(anyString(), anyLong(), anyLong(),
- anyBoolean(), anyBoolean(), anyBoolean())).
- thenThrow(new FileNotFoundException()) ;
+ when(fsName.getBlockLocations(
+ anyString(), anyLong(), anyLong(), anyBoolean(), anyBoolean()))
+ .thenThrow(new FileNotFoundException());
when(fsName.getBlockManager()).thenReturn(blockManager);
when(blockManager.getDatanodeManager()).thenReturn(dnManager);