You are viewing a plain text version of this content. The canonical link for it is here.
Posted to hdfs-commits@hadoop.apache.org by jg...@apache.org on 2010/07/20 23:25:22 UTC
svn commit: r966016 - in /hadoop/hdfs/trunk: ./
src/java/org/apache/hadoop/hdfs/protocol/
src/java/org/apache/hadoop/hdfs/security/token/block/
src/java/org/apache/hadoop/hdfs/server/namenode/
src/java/org/apache/hadoop/hdfs/server/protocol/ src/test/h...
Author: jghoman
Date: Tue Jul 20 21:25:21 2010
New Revision: 966016
URL: http://svn.apache.org/viewvc?rev=966016&view=rev
Log:
HDFS-1081. Performance regression in DistributedFileSystem::getFileBlockLocations in secure systems.
Modified:
hadoop/hdfs/trunk/CHANGES.txt
hadoop/hdfs/trunk/src/java/org/apache/hadoop/hdfs/protocol/ClientProtocol.java
hadoop/hdfs/trunk/src/java/org/apache/hadoop/hdfs/security/token/block/BlockTokenIdentifier.java
hadoop/hdfs/trunk/src/java/org/apache/hadoop/hdfs/security/token/block/BlockTokenSecretManager.java
hadoop/hdfs/trunk/src/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
hadoop/hdfs/trunk/src/java/org/apache/hadoop/hdfs/server/protocol/DatanodeProtocol.java
hadoop/hdfs/trunk/src/test/hdfs/org/apache/hadoop/hdfs/security/token/block/TestBlockToken.java
Modified: hadoop/hdfs/trunk/CHANGES.txt
URL: http://svn.apache.org/viewvc/hadoop/hdfs/trunk/CHANGES.txt?rev=966016&r1=966015&r2=966016&view=diff
==============================================================================
--- hadoop/hdfs/trunk/CHANGES.txt (original)
+++ hadoop/hdfs/trunk/CHANGES.txt Tue Jul 20 21:25:21 2010
@@ -87,6 +87,9 @@ Trunk (unreleased changes)
HDFS-1140. Speedup INode.getPathComponents. (Dmytro Molkov via shv)
+ HDFS-1081. Performance regression in
+ DistributedFileSystem::getFileBlockLocations in secure systems (jghoman)
+
BUG FIXES
HDFS-1039. Adding test for JspHelper.getUGI(jnp via boryas)
Modified: hadoop/hdfs/trunk/src/java/org/apache/hadoop/hdfs/protocol/ClientProtocol.java
URL: http://svn.apache.org/viewvc/hadoop/hdfs/trunk/src/java/org/apache/hadoop/hdfs/protocol/ClientProtocol.java?rev=966016&r1=966015&r2=966016&view=diff
==============================================================================
--- hadoop/hdfs/trunk/src/java/org/apache/hadoop/hdfs/protocol/ClientProtocol.java (original)
+++ hadoop/hdfs/trunk/src/java/org/apache/hadoop/hdfs/protocol/ClientProtocol.java Tue Jul 20 21:25:21 2010
@@ -68,9 +68,9 @@ public interface ClientProtocol extends
* Compared to the previous version the following changes have been introduced:
* (Only the latest change is reflected.
* The log of historical changes can be retrieved from the svn).
- * 60: Replace full getListing with iterative getListinng.
+ * 61: HDFS-1081. Performance optimization on getBlocksLocation().
*/
- public static final long versionID = 60L;
+ public static final long versionID = 61L;
///////////////////////////////////////
// File contents
Modified: hadoop/hdfs/trunk/src/java/org/apache/hadoop/hdfs/security/token/block/BlockTokenIdentifier.java
URL: http://svn.apache.org/viewvc/hadoop/hdfs/trunk/src/java/org/apache/hadoop/hdfs/security/token/block/BlockTokenIdentifier.java?rev=966016&r1=966015&r2=966016&view=diff
==============================================================================
--- hadoop/hdfs/trunk/src/java/org/apache/hadoop/hdfs/security/token/block/BlockTokenIdentifier.java (original)
+++ hadoop/hdfs/trunk/src/java/org/apache/hadoop/hdfs/security/token/block/BlockTokenIdentifier.java Tue Jul 20 21:25:21 2010
@@ -21,6 +21,7 @@ package org.apache.hadoop.hdfs.security.
import java.io.DataInput;
import java.io.DataOutput;
import java.io.IOException;
+import java.util.Arrays;
import java.util.EnumSet;
import org.apache.hadoop.classification.InterfaceAudience;
@@ -37,17 +38,23 @@ public class BlockTokenIdentifier extend
private long expiryDate;
private int keyId;
private String userId;
- private long blockId;
+ private long [] blockIds;
private EnumSet<AccessMode> modes;
+ private byte [] cache;
+
public BlockTokenIdentifier() {
- this(null, 0, EnumSet.noneOf(AccessMode.class));
+ this(null, new long [] {}, EnumSet.noneOf(AccessMode.class));
}
- public BlockTokenIdentifier(String userId, long blockId,
+ public BlockTokenIdentifier(String userId, long [] blockIds,
EnumSet<AccessMode> modes) {
+ if(blockIds == null)
+ throw new IllegalArgumentException("blockIds can't be null");
+ this.cache = null;
this.userId = userId;
- this.blockId = blockId;
+ this.blockIds = Arrays.copyOf(blockIds, blockIds.length);
+ Arrays.sort(this.blockIds);
this.modes = modes == null ? EnumSet.noneOf(AccessMode.class) : modes;
}
@@ -59,7 +66,7 @@ public class BlockTokenIdentifier extend
@Override
public UserGroupInformation getUser() {
if (userId == null || "".equals(userId)) {
- return UserGroupInformation.createRemoteUser(Long.toString(blockId));
+ return UserGroupInformation.createRemoteUser(Arrays.toString(blockIds));
}
return UserGroupInformation.createRemoteUser(userId);
}
@@ -69,6 +76,7 @@ public class BlockTokenIdentifier extend
}
public void setExpiryDate(long expiryDate) {
+ this.cache = null;
this.expiryDate = expiryDate;
}
@@ -77,6 +85,7 @@ public class BlockTokenIdentifier extend
}
public void setKeyId(int keyId) {
+ this.cache = null;
this.keyId = keyId;
}
@@ -84,18 +93,36 @@ public class BlockTokenIdentifier extend
return userId;
}
- public long getBlockId() {
- return blockId;
+ /**
+ * Return sorted array of blockIds this {@link BlockTokenIdentifier} includes
+ */
+ public long [] getBlockIds() {
+ return blockIds;
+ }
+
+ /**
+ * Is specified blockId included in this BlockTokenIdentifier?
+ */
+ public boolean isBlockIncluded(long blockId) {
+ switch(blockIds.length) {
+ case 1:
+ return blockIds[0] == blockId;
+ case 2:
+ return (blockIds[0] == blockId) || (blockIds[1] == blockId);
+ default:
+ return Arrays.binarySearch(blockIds, blockId) >= 0;
+ }
}
-
+
public EnumSet<AccessMode> getAccessModes() {
return modes;
}
+ @Override
public String toString() {
return "block_token_identifier (expiryDate=" + this.getExpiryDate()
+ ", keyId=" + this.getKeyId() + ", userId=" + this.getUserId()
- + ", blockId=" + this.getBlockId() + ", access modes="
+ + ", blockId=" + Arrays.toString(blockIds) + ", access modes="
+ this.getAccessModes() + ")";
}
@@ -111,7 +138,8 @@ public class BlockTokenIdentifier extend
if (obj instanceof BlockTokenIdentifier) {
BlockTokenIdentifier that = (BlockTokenIdentifier) obj;
return this.expiryDate == that.expiryDate && this.keyId == that.keyId
- && isEqual(this.userId, that.userId) && this.blockId == that.blockId
+ && isEqual(this.userId, that.userId)
+ && Arrays.equals(this.blockIds, that.blockIds)
&& isEqual(this.modes, that.modes);
}
return false;
@@ -119,15 +147,18 @@ public class BlockTokenIdentifier extend
/** {@inheritDoc} */
public int hashCode() {
- return (int) expiryDate ^ keyId ^ (int) blockId ^ modes.hashCode()
+ return (int) expiryDate ^ keyId ^ Arrays.hashCode(blockIds) ^ modes.hashCode()
^ (userId == null ? 0 : userId.hashCode());
}
public void readFields(DataInput in) throws IOException {
+ cache = null;
expiryDate = WritableUtils.readVLong(in);
keyId = WritableUtils.readVInt(in);
userId = WritableUtils.readString(in);
- blockId = WritableUtils.readVLong(in);
+ blockIds = new long[WritableUtils.readVInt(in)];
+ for(int i = 0; i < blockIds.length; i++)
+ blockIds[i] = WritableUtils.readVLong(in);
int length = WritableUtils.readVInt(in);
for (int i = 0; i < length; i++) {
modes.add(WritableUtils.readEnum(in, AccessMode.class));
@@ -138,10 +169,19 @@ public class BlockTokenIdentifier extend
WritableUtils.writeVLong(out, expiryDate);
WritableUtils.writeVInt(out, keyId);
WritableUtils.writeString(out, userId);
- WritableUtils.writeVLong(out, blockId);
+ WritableUtils.writeVInt(out, blockIds.length);
+ for(int i = 0; i < blockIds.length; i++)
+ WritableUtils.writeVLong(out, blockIds[i]);
WritableUtils.writeVInt(out, modes.size());
for (AccessMode aMode : modes) {
WritableUtils.writeEnum(out, aMode);
}
}
+
+ @Override
+ public byte[] getBytes() {
+ if(cache == null) cache = super.getBytes();
+
+ return cache;
+ }
}
Modified: hadoop/hdfs/trunk/src/java/org/apache/hadoop/hdfs/security/token/block/BlockTokenSecretManager.java
URL: http://svn.apache.org/viewvc/hadoop/hdfs/trunk/src/java/org/apache/hadoop/hdfs/security/token/block/BlockTokenSecretManager.java?rev=966016&r1=966015&r2=966016&view=diff
==============================================================================
--- hadoop/hdfs/trunk/src/java/org/apache/hadoop/hdfs/security/token/block/BlockTokenSecretManager.java (original)
+++ hadoop/hdfs/trunk/src/java/org/apache/hadoop/hdfs/security/token/block/BlockTokenSecretManager.java Tue Jul 20 21:25:21 2010
@@ -176,16 +176,29 @@ public class BlockTokenSecretManager ext
/** Generate an block token for current user */
public Token<BlockTokenIdentifier> generateToken(Block block,
EnumSet<AccessMode> modes) throws IOException {
- UserGroupInformation ugi = UserGroupInformation.getCurrentUser();
- String userID = (ugi == null ? null : ugi.getShortUserName());
- return generateToken(userID, block, modes);
+ return generateToken(new long [] { block.getBlockId() }, modes);
}
/** Generate a block token for a specified user */
public Token<BlockTokenIdentifier> generateToken(String userId, Block block,
EnumSet<AccessMode> modes) throws IOException {
- BlockTokenIdentifier id = new BlockTokenIdentifier(userId, block
- .getBlockId(), modes);
+ return generateToken(userId, new long [] { block.getBlockId() }, modes);
+ }
+
+ /** Generate a block token for the current user based on a collection
+ * of blockIds
+ */
+ public Token<BlockTokenIdentifier> generateToken(long[] blockIds,
+ EnumSet<AccessMode> modes) throws IOException {
+ UserGroupInformation ugi = UserGroupInformation.getCurrentUser();
+ String userID = (ugi == null ? null : ugi.getShortUserName());
+ return generateToken(userID, blockIds, modes);
+ }
+
+ /** Generate a block token based on a collection of blockIds */
+ public Token<BlockTokenIdentifier> generateToken(String userID,
+ long[] blockIds, EnumSet<AccessMode> modes) {
+ BlockTokenIdentifier id = new BlockTokenIdentifier(userID, blockIds, modes);
return new Token<BlockTokenIdentifier>(id, this);
}
@@ -204,7 +217,7 @@ public class BlockTokenSecretManager ext
throw new InvalidToken("Block token with " + id.toString()
+ " doesn't belong to user " + userId);
}
- if (id.getBlockId() != block.getBlockId()) {
+ if (!id.isBlockIncluded(block.getBlockId())) {
throw new InvalidToken("Block token with " + id.toString()
+ " doesn't apply to block " + block);
}
Modified: hadoop/hdfs/trunk/src/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
URL: http://svn.apache.org/viewvc/hadoop/hdfs/trunk/src/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java?rev=966016&r1=966015&r2=966016&view=diff
==============================================================================
--- hadoop/hdfs/trunk/src/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java (original)
+++ hadoop/hdfs/trunk/src/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java Tue Jul 20 21:25:21 2010
@@ -24,6 +24,7 @@ import org.apache.hadoop.classification.
import org.apache.hadoop.conf.*;
import org.apache.hadoop.hdfs.DFSUtil;
import org.apache.hadoop.hdfs.protocol.*;
+import org.apache.hadoop.hdfs.security.token.block.BlockTokenIdentifier;
import org.apache.hadoop.hdfs.security.token.block.BlockTokenSecretManager;
import org.apache.hadoop.hdfs.security.token.block.ExportedBlockKeys;
import org.apache.hadoop.hdfs.server.common.GenerationStamp;
@@ -792,26 +793,36 @@ public class FSNamesystem implements FSC
LOG.debug("last = " + last);
}
- if (!last.isComplete()) {
- return new LocatedBlocks(n, inode.isUnderConstruction(), locatedblocks,
- blockManager.getBlockLocation(last, n), false);
- }
- else {
+ if(isBlockTokenEnabled) setBlockTokens(locatedblocks);
+
+ if (last.isComplete()) {
return new LocatedBlocks(n, inode.isUnderConstruction(), locatedblocks,
blockManager.getBlockLocation(last, n-last.getNumBytes()), true);
+ } else {
+ return new LocatedBlocks(n, inode.isUnderConstruction(), locatedblocks,
+ blockManager.getBlockLocation(last, n), false);
}
}
}
+ /** Generate a combined block token for all the blocks to be returned. */
+ private void setBlockTokens(List<LocatedBlock> locatedBlocks) throws IOException {
+ long [] blockIds = new long[locatedBlocks.size()];
+ for(int i = 0; i < blockIds.length; i++) {
+ blockIds[i] = locatedBlocks.get(i).getBlock().getBlockId();
+ }
+
+ Token<BlockTokenIdentifier> token =
+ blockTokenSecretManager.generateToken(blockIds,
+ EnumSet.of(BlockTokenSecretManager.AccessMode.READ));
+
+ for(LocatedBlock l : locatedBlocks) l.setBlockToken(token);
+ }
+
/** Create a LocatedBlock. */
LocatedBlock createLocatedBlock(final Block b, final DatanodeInfo[] locations,
final long offset, final boolean corrupt) throws IOException {
- final LocatedBlock lb = new LocatedBlock(b, locations, offset, corrupt);
- if (isBlockTokenEnabled) {
- lb.setBlockToken(blockTokenSecretManager.generateToken(b,
- EnumSet.of(BlockTokenSecretManager.AccessMode.READ)));
- }
- return lb;
+ return new LocatedBlock(b, locations, offset, corrupt);
}
/**
Modified: hadoop/hdfs/trunk/src/java/org/apache/hadoop/hdfs/server/protocol/DatanodeProtocol.java
URL: http://svn.apache.org/viewvc/hadoop/hdfs/trunk/src/java/org/apache/hadoop/hdfs/server/protocol/DatanodeProtocol.java?rev=966016&r1=966015&r2=966016&view=diff
==============================================================================
--- hadoop/hdfs/trunk/src/java/org/apache/hadoop/hdfs/server/protocol/DatanodeProtocol.java (original)
+++ hadoop/hdfs/trunk/src/java/org/apache/hadoop/hdfs/server/protocol/DatanodeProtocol.java Tue Jul 20 21:25:21 2010
@@ -44,9 +44,9 @@ import org.apache.avro.reflect.Nullable;
@InterfaceAudience.Private
public interface DatanodeProtocol extends VersionedProtocol {
/**
- * 24: register() renamed registerDatanode()
+ * 25: HDFS-1081. Performance optimization on getBlocksLocation()
*/
- public static final long versionID = 24L;
+ public static final long versionID = 25L;
// error code
final static int NOTIFY = 0;
Modified: hadoop/hdfs/trunk/src/test/hdfs/org/apache/hadoop/hdfs/security/token/block/TestBlockToken.java
URL: http://svn.apache.org/viewvc/hadoop/hdfs/trunk/src/test/hdfs/org/apache/hadoop/hdfs/security/token/block/TestBlockToken.java?rev=966016&r1=966015&r2=966016&view=diff
==============================================================================
--- hadoop/hdfs/trunk/src/test/hdfs/org/apache/hadoop/hdfs/security/token/block/TestBlockToken.java (original)
+++ hadoop/hdfs/trunk/src/test/hdfs/org/apache/hadoop/hdfs/security/token/block/TestBlockToken.java Tue Jul 20 21:25:21 2010
@@ -22,6 +22,7 @@ import java.io.ByteArrayInputStream;
import java.io.DataInputStream;
import java.io.IOException;
import java.net.InetSocketAddress;
+import java.util.Arrays;
import java.util.EnumSet;
import java.util.Set;
@@ -102,15 +103,16 @@ public class TestBlockToken {
Set<TokenIdentifier> tokenIds = UserGroupInformation.getCurrentUser()
.getTokenIdentifiers();
assertEquals("Only one BlockTokenIdentifier expected", 1, tokenIds.size());
- long result = 0;
+ long [] result = {0};
for (TokenIdentifier tokenId : tokenIds) {
BlockTokenIdentifier id = (BlockTokenIdentifier) tokenId;
LOG.info("Got: " + id.toString());
assertTrue("Received BlockTokenIdentifier is wrong", ident.equals(id));
sm.checkAccess(id, null, block, BlockTokenSecretManager.AccessMode.WRITE);
- result = id.getBlockId();
+ result = id.getBlockIds();
}
- return result;
+ assertEquals("Got more than one block back", 1, result.length);
+ return result[0];
}
}
@@ -221,4 +223,28 @@ public class TestBlockToken {
}
}
+ @Test
+ public void collectionOfBlocksActsSanely() {
+ final long[][] testBlockIds = new long [][] {{99l, 7l, -32l, 0l},
+ {},
+ {42l},
+ {-5235l, 2352}};
+ final long [] notBlockIds = new long [] { 32l, 1l, -23423423l};
+
+ for(long [] bids : testBlockIds) {
+ BlockTokenIdentifier bti = new BlockTokenIdentifier("Madame Butterfly",
+ bids, EnumSet.noneOf(BlockTokenSecretManager.AccessMode.class));
+
+ for(long bid : bids) assertTrue(bti.isBlockIncluded(bid));
+
+ for(long nbid : notBlockIds) assertFalse(bti.isBlockIncluded(nbid));
+
+ // BlockTokenIdentifiers maintain a sorted array of the block Ids.
+ long[] sorted = Arrays.copyOf(bids, bids.length);
+ Arrays.sort(sorted);
+
+ assertTrue(Arrays.toString(bids)+" doesn't equal "+Arrays.toString(sorted),
+ Arrays.equals(bti.getBlockIds(), sorted));
+ }
+ }
}