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 ha...@apache.org on 2018/05/23 17:12:57 UTC
[09/18] hadoop git commit: HDFS-13601. Optimize ByteString
conversions in PBHelper.
HDFS-13601. Optimize ByteString conversions in PBHelper.
Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo
Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/1d2640b6
Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/1d2640b6
Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/1d2640b6
Branch: refs/heads/HDDS-48
Commit: 1d2640b6132e8308c07476badd2d1482be68a298
Parents: 5a91406
Author: Andrew Wang <wa...@apache.org>
Authored: Tue May 22 23:55:20 2018 -0700
Committer: Andrew Wang <wa...@apache.org>
Committed: Tue May 22 23:55:20 2018 -0700
----------------------------------------------------------------------
.../dev-support/findbugsExcludeFile.xml | 5 ++
.../apache/hadoop/hdfs/protocol/DatanodeID.java | 50 +++++++++++++--
.../hadoop/hdfs/protocolPB/PBHelperClient.java | 67 +++++++++++++++++---
.../TestDataXceiverBackwardsCompat.java | 10 +++
4 files changed, 118 insertions(+), 14 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/hadoop/blob/1d2640b6/hadoop-hdfs-project/hadoop-hdfs-client/dev-support/findbugsExcludeFile.xml
----------------------------------------------------------------------
diff --git a/hadoop-hdfs-project/hadoop-hdfs-client/dev-support/findbugsExcludeFile.xml b/hadoop-hdfs-project/hadoop-hdfs-client/dev-support/findbugsExcludeFile.xml
index 8e2bc94..fa9654b 100644
--- a/hadoop-hdfs-project/hadoop-hdfs-client/dev-support/findbugsExcludeFile.xml
+++ b/hadoop-hdfs-project/hadoop-hdfs-client/dev-support/findbugsExcludeFile.xml
@@ -91,5 +91,10 @@
<Method name="getSymlinkInBytes" />
<Bug pattern="EI_EXPOSE_REP" />
</Match>
+ <Match>
+ <Class name="org.apache.hadoop.hdfs.protocolPB.PBHelperClient" />
+ <Method name="getFixedByteString" />
+ <Bug pattern="AT_OPERATION_SEQUENCE_ON_CONCURRENT_ABSTRACTION" />
+ </Match>
</FindBugsFilter>
http://git-wip-us.apache.org/repos/asf/hadoop/blob/1d2640b6/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/protocol/DatanodeID.java
----------------------------------------------------------------------
diff --git a/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/protocol/DatanodeID.java b/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/protocol/DatanodeID.java
index af720c7..718661e 100644
--- a/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/protocol/DatanodeID.java
+++ b/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/protocol/DatanodeID.java
@@ -18,6 +18,7 @@
package org.apache.hadoop.hdfs.protocol;
+import com.google.protobuf.ByteString;
import org.apache.hadoop.classification.InterfaceAudience;
import org.apache.hadoop.classification.InterfaceStability;
@@ -44,7 +45,9 @@ public class DatanodeID implements Comparable<DatanodeID> {
"null", "null", 0, 0, 0, 0);
private String ipAddr; // IP address
+ private ByteString ipAddrBytes; // ipAddr ByteString to save on PB serde
private String hostName; // hostname claimed by datanode
+ private ByteString hostNameBytes; // hostName ByteString to save on PB serde
private String peerHostName; // hostname from the actual connection
private int xferPort; // data streaming port
private int infoPort; // info server port
@@ -58,6 +61,8 @@ public class DatanodeID implements Comparable<DatanodeID> {
* For newly formatted Datanodes it is a UUID.
*/
private final String datanodeUuid;
+ // datanodeUuid ByteString to save on PB serde
+ private final ByteString datanodeUuidBytes;
public DatanodeID(DatanodeID from) {
this(from.getDatanodeUuid(), from);
@@ -66,8 +71,11 @@ public class DatanodeID implements Comparable<DatanodeID> {
@VisibleForTesting
public DatanodeID(String datanodeUuid, DatanodeID from) {
this(from.getIpAddr(),
+ from.getIpAddrBytes(),
from.getHostName(),
+ from.getHostNameBytes(),
datanodeUuid,
+ getByteString(datanodeUuid),
from.getXferPort(),
from.getInfoPort(),
from.getInfoSecurePort(),
@@ -89,22 +97,43 @@ public class DatanodeID implements Comparable<DatanodeID> {
*/
public DatanodeID(String ipAddr, String hostName, String datanodeUuid,
int xferPort, int infoPort, int infoSecurePort, int ipcPort) {
- setIpAndXferPort(ipAddr, xferPort);
+ this(ipAddr, getByteString(ipAddr),
+ hostName, getByteString(hostName),
+ datanodeUuid, getByteString(datanodeUuid),
+ xferPort, infoPort, infoSecurePort, ipcPort);
+ }
+
+ private DatanodeID(String ipAddr, ByteString ipAddrBytes,
+ String hostName, ByteString hostNameBytes,
+ String datanodeUuid, ByteString datanodeUuidBytes,
+ int xferPort, int infoPort, int infoSecurePort, int ipcPort) {
+ setIpAndXferPort(ipAddr, ipAddrBytes, xferPort);
this.hostName = hostName;
+ this.hostNameBytes = hostNameBytes;
this.datanodeUuid = checkDatanodeUuid(datanodeUuid);
+ this.datanodeUuidBytes = datanodeUuidBytes;
this.infoPort = infoPort;
this.infoSecurePort = infoSecurePort;
this.ipcPort = ipcPort;
}
+ private static ByteString getByteString(String str) {
+ if (str != null) {
+ return ByteString.copyFromUtf8(str);
+ }
+ return ByteString.EMPTY;
+ }
+
public void setIpAddr(String ipAddr) {
//updated during registration, preserve former xferPort
- setIpAndXferPort(ipAddr, xferPort);
+ setIpAndXferPort(ipAddr, getByteString(ipAddr), xferPort);
}
- private void setIpAndXferPort(String ipAddr, int xferPort) {
+ private void setIpAndXferPort(String ipAddr, ByteString ipAddrBytes,
+ int xferPort) {
// build xferAddr string to reduce cost of frequent use
this.ipAddr = ipAddr;
+ this.ipAddrBytes = ipAddrBytes;
this.xferPort = xferPort;
this.xferAddr = ipAddr + ":" + xferPort;
}
@@ -120,6 +149,10 @@ public class DatanodeID implements Comparable<DatanodeID> {
return datanodeUuid;
}
+ public ByteString getDatanodeUuidBytes() {
+ return datanodeUuidBytes;
+ }
+
private String checkDatanodeUuid(String uuid) {
if (uuid == null || uuid.isEmpty()) {
return null;
@@ -135,6 +168,10 @@ public class DatanodeID implements Comparable<DatanodeID> {
return ipAddr;
}
+ public ByteString getIpAddrBytes() {
+ return ipAddrBytes;
+ }
+
/**
* @return hostname
*/
@@ -142,6 +179,10 @@ public class DatanodeID implements Comparable<DatanodeID> {
return hostName;
}
+ public ByteString getHostNameBytes() {
+ return hostNameBytes;
+ }
+
/**
* @return hostname from the actual connection
*/
@@ -258,7 +299,8 @@ public class DatanodeID implements Comparable<DatanodeID> {
* Note that this does not update storageID.
*/
public void updateRegInfo(DatanodeID nodeReg) {
- setIpAndXferPort(nodeReg.getIpAddr(), nodeReg.getXferPort());
+ setIpAndXferPort(nodeReg.getIpAddr(), nodeReg.getIpAddrBytes(),
+ nodeReg.getXferPort());
hostName = nodeReg.getHostName();
peerHostName = nodeReg.getPeerHostName();
infoPort = nodeReg.getInfoPort();
http://git-wip-us.apache.org/repos/asf/hadoop/blob/1d2640b6/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/protocolPB/PBHelperClient.java
----------------------------------------------------------------------
diff --git a/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/protocolPB/PBHelperClient.java b/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/protocolPB/PBHelperClient.java
index ff9733c..579ac43 100644
--- a/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/protocolPB/PBHelperClient.java
+++ b/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/protocolPB/PBHelperClient.java
@@ -27,8 +27,12 @@ import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
import com.google.common.base.Preconditions;
+import com.google.common.cache.CacheBuilder;
+import com.google.common.cache.CacheLoader;
+import com.google.common.cache.LoadingCache;
import com.google.common.collect.Lists;
import com.google.common.primitives.Shorts;
import com.google.protobuf.ByteString;
@@ -228,6 +232,49 @@ public class PBHelperClient {
private static final FsAction[] FSACTION_VALUES =
FsAction.values();
+ /**
+ * Map used to cache fixed strings to ByteStrings. Since there is no
+ * automatic expiration policy, only use this for strings from a fixed, small
+ * set.
+ * <p/>
+ * This map should not be accessed directly. Used the getFixedByteString
+ * methods instead.
+ */
+ private static ConcurrentHashMap<Object, ByteString> fixedByteStringCache =
+ new ConcurrentHashMap<>();
+
+ private static ByteString getFixedByteString(Text key) {
+ ByteString value = fixedByteStringCache.get(key);
+ if (value == null) {
+ value = ByteString.copyFromUtf8(key.toString());
+ fixedByteStringCache.put(key, value);
+ }
+ return value;
+ }
+
+ private static ByteString getFixedByteString(String key) {
+ ByteString value = fixedByteStringCache.get(key);
+ if (value == null) {
+ value = ByteString.copyFromUtf8(key);
+ fixedByteStringCache.put(key, value);
+ }
+ return value;
+ }
+
+ /**
+ * Guava cache for caching String to ByteString encoding. Use this when the
+ * set of Strings is large, mutable, or unknown.
+ */
+ private static LoadingCache<String, ByteString> bytestringCache =
+ CacheBuilder.newBuilder()
+ .maximumSize(10000)
+ .build(
+ new CacheLoader<String, ByteString>() {
+ public ByteString load(String key) {
+ return ByteString.copyFromUtf8(key);
+ }
+ });
+
private PBHelperClient() {
/** Hidden constructor */
}
@@ -294,7 +341,7 @@ public class PBHelperClient {
public static ExtendedBlockProto convert(final ExtendedBlock b) {
if (b == null) return null;
return ExtendedBlockProto.newBuilder().
- setPoolId(b.getBlockPoolId()).
+ setPoolIdBytes(getFixedByteString(b.getBlockPoolId())).
setBlockId(b.getBlockId()).
setNumBytes(b.getNumBytes()).
setGenerationStamp(b.getGenerationStamp()).
@@ -305,8 +352,8 @@ public class PBHelperClient {
return TokenProto.newBuilder().
setIdentifier(getByteString(tok.getIdentifier())).
setPassword(getByteString(tok.getPassword())).
- setKind(tok.getKind().toString()).
- setService(tok.getService().toString()).build();
+ setKindBytes(getFixedByteString(tok.getKind())).
+ setServiceBytes(getFixedByteString(tok.getService())).build();
}
public static ShortCircuitShmIdProto convert(ShmId shmId) {
@@ -329,11 +376,10 @@ public class PBHelperClient {
// which is the same as the DatanodeUuid. Since StorageID is a required
// field we pass the empty string if the DatanodeUuid is not yet known.
return DatanodeIDProto.newBuilder()
- .setIpAddr(dn.getIpAddr())
- .setHostName(dn.getHostName())
+ .setIpAddrBytes(dn.getIpAddrBytes())
+ .setHostNameBytes(dn.getHostNameBytes())
.setXferPort(dn.getXferPort())
- .setDatanodeUuid(dn.getDatanodeUuid() != null ?
- dn.getDatanodeUuid() : "")
+ .setDatanodeUuidBytes(dn.getDatanodeUuidBytes())
.setInfoPort(dn.getInfoPort())
.setInfoSecurePort(dn.getInfoSecurePort())
.setIpcPort(dn.getIpcPort()).build();
@@ -357,7 +403,8 @@ public class PBHelperClient {
public static DatanodeInfoProto convert(DatanodeInfo info) {
DatanodeInfoProto.Builder builder = DatanodeInfoProto.newBuilder();
if (info.getNetworkLocation() != null) {
- builder.setLocation(info.getNetworkLocation());
+ builder.setLocationBytes(
+ bytestringCache.getUnchecked(info.getNetworkLocation()));
}
if (info.getUpgradeDomain() != null) {
builder.setUpgradeDomain(info.getUpgradeDomain());
@@ -2260,8 +2307,8 @@ public class PBHelperClient {
setModificationTime(fs.getModificationTime()).
setAccessTime(fs.getAccessTime()).
setPermission(convert(fs.getPermission())).
- setOwner(fs.getOwner()).
- setGroup(fs.getGroup()).
+ setOwnerBytes(getFixedByteString(fs.getOwner())).
+ setGroupBytes(getFixedByteString(fs.getGroup())).
setFileId(fs.getFileId()).
setChildrenNum(fs.getChildrenNum()).
setPath(getByteString(fs.getLocalNameInBytes())).
http://git-wip-us.apache.org/repos/asf/hadoop/blob/1d2640b6/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestDataXceiverBackwardsCompat.java
----------------------------------------------------------------------
diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestDataXceiverBackwardsCompat.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestDataXceiverBackwardsCompat.java
index bdcbe7f..0f65269 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestDataXceiverBackwardsCompat.java
+++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/datanode/TestDataXceiverBackwardsCompat.java
@@ -18,6 +18,7 @@
package org.apache.hadoop.hdfs.server.datanode;
+import com.google.protobuf.ByteString;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.fs.StorageType;
import org.apache.hadoop.hdfs.net.*;
@@ -47,6 +48,7 @@ import java.io.OutputStream;
import java.io.PrintStream;
import java.net.ServerSocket;
import java.net.Socket;
+import java.util.UUID;
import static org.junit.Assert.fail;
import static org.mockito.Mockito.*;
@@ -171,9 +173,17 @@ public class TestDataXceiverBackwardsCompat {
DatanodeInfo datanodeInfo = mock(DatanodeInfo.class);
doReturn("localhost").when(datanodeInfo).getHostName();
+ doReturn(ByteString.copyFromUtf8("localhost"))
+ .when(datanodeInfo).getHostNameBytes();
doReturn("127.0.0.1").when(datanodeInfo).getIpAddr();
+ doReturn(ByteString.copyFromUtf8("127.0.0.1"))
+ .when(datanodeInfo).getIpAddrBytes();
doReturn(DatanodeInfo.AdminStates.NORMAL).when(datanodeInfo)
.getAdminState();
+ final String uuid = UUID.randomUUID().toString();
+ doReturn(uuid).when(datanodeInfo).getDatanodeUuid();
+ doReturn(ByteString.copyFromUtf8(uuid))
+ .when(datanodeInfo).getDatanodeUuidBytes();
Exception storedException = null;
try {
---------------------------------------------------------------------
To unsubscribe, e-mail: common-commits-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-commits-help@hadoop.apache.org