Skip to content

Commit edbbc03

Browse files
authored
HADOOP-16621. [pb-upgrade] Remove Protobuf classes from signatures of Public APIs. Contributed by Vinayakumar B. (#1803)
1 parent a0ff42d commit edbbc03

File tree

6 files changed

+84
-74
lines changed

6 files changed

+84
-74
lines changed

hadoop-common-project/hadoop-common/dev-support/findbugsExcludeFile.xml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -460,4 +460,10 @@
460460
<Method name="save"/>
461461
<Bug pattern="OBL_UNSATISFIED_OBLIGATION"/>
462462
</Match>
463+
464+
<Match>
465+
<Class name="org.apache.hadoop.ipc.ProtobufHelper" />
466+
<Method name="getFixedByteString" />
467+
<Bug pattern="AT_OPERATION_SEQUENCE_ON_CONCURRENT_ABSTRACTION" />
468+
</Match>
463469
</FindBugsFilter>

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/ProtobufHelper.java

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,15 @@
1818
package org.apache.hadoop.ipc;
1919

2020
import java.io.IOException;
21+
import java.util.concurrent.ConcurrentHashMap;
2122

2223
import org.apache.hadoop.classification.InterfaceAudience;
24+
import org.apache.hadoop.io.Text;
25+
import org.apache.hadoop.security.proto.SecurityProtos.TokenProto;
26+
import org.apache.hadoop.security.token.Token;
27+
import org.apache.hadoop.security.token.TokenIdentifier;
2328

29+
import com.google.protobuf.ByteString;
2430
import com.google.protobuf.ServiceException;
2531

2632
/**
@@ -46,4 +52,67 @@ public static IOException getRemoteException(ServiceException se) {
4652
}
4753
return e instanceof IOException ? (IOException) e : new IOException(se);
4854
}
55+
56+
57+
/**
58+
* Map used to cache fixed strings to ByteStrings. Since there is no
59+
* automatic expiration policy, only use this for strings from a fixed, small
60+
* set.
61+
* <p/>
62+
* This map should not be accessed directly. Used the getFixedByteString
63+
* methods instead.
64+
*/
65+
private final static ConcurrentHashMap<Object, ByteString>
66+
FIXED_BYTESTRING_CACHE = new ConcurrentHashMap<>();
67+
68+
/**
69+
* Get the ByteString for frequently used fixed and small set strings.
70+
* @param key string
71+
* @return
72+
*/
73+
public static ByteString getFixedByteString(Text key) {
74+
ByteString value = FIXED_BYTESTRING_CACHE.get(key);
75+
if (value == null) {
76+
value = ByteString.copyFromUtf8(key.toString());
77+
FIXED_BYTESTRING_CACHE.put(new Text(key.copyBytes()), value);
78+
}
79+
return value;
80+
}
81+
82+
/**
83+
* Get the ByteString for frequently used fixed and small set strings.
84+
* @param key string
85+
* @return
86+
*/
87+
public static ByteString getFixedByteString(String key) {
88+
ByteString value = FIXED_BYTESTRING_CACHE.get(key);
89+
if (value == null) {
90+
value = ByteString.copyFromUtf8(key);
91+
FIXED_BYTESTRING_CACHE.put(key, value);
92+
}
93+
return value;
94+
}
95+
96+
public static ByteString getByteString(byte[] bytes) {
97+
// return singleton to reduce object allocation
98+
return (bytes.length == 0) ? ByteString.EMPTY : ByteString.copyFrom(bytes);
99+
}
100+
101+
public static Token<? extends TokenIdentifier> tokenFromProto(
102+
TokenProto tokenProto) {
103+
Token<? extends TokenIdentifier> token = new Token<>(
104+
tokenProto.getIdentifier().toByteArray(),
105+
tokenProto.getPassword().toByteArray(), new Text(tokenProto.getKind()),
106+
new Text(tokenProto.getService()));
107+
return token;
108+
}
109+
110+
public static TokenProto protoFromToken(Token<?> tok) {
111+
TokenProto.Builder builder = TokenProto.newBuilder().
112+
setIdentifier(getByteString(tok.getIdentifier())).
113+
setPassword(getByteString(tok.getPassword())).
114+
setKindBytes(getFixedByteString(tok.getKind())).
115+
setServiceBytes(getFixedByteString(tok.getService()));
116+
return builder.build();
117+
}
49118
}

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/Credentials.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@
4646
import org.apache.hadoop.io.Text;
4747
import org.apache.hadoop.io.Writable;
4848
import org.apache.hadoop.io.WritableUtils;
49+
import org.apache.hadoop.ipc.ProtobufHelper;
4950
import org.apache.hadoop.security.token.Token;
5051
import org.apache.hadoop.security.token.TokenIdentifier;
5152
import org.apache.hadoop.security.proto.SecurityProtos.CredentialsKVProto;
@@ -368,7 +369,7 @@ void writeProto(DataOutput out) throws IOException {
368369
CredentialsKVProto.Builder kv = CredentialsKVProto.newBuilder().
369370
setAliasBytes(ByteString.copyFrom(
370371
e.getKey().getBytes(), 0, e.getKey().getLength())).
371-
setToken(e.getValue().toTokenProto());
372+
setToken(ProtobufHelper.protoFromToken(e.getValue()));
372373
storage.addTokens(kv.build());
373374
}
374375

@@ -390,7 +391,7 @@ void readProto(DataInput in) throws IOException {
390391
CredentialsProto storage = CredentialsProto.parseDelimitedFrom((DataInputStream)in);
391392
for (CredentialsKVProto kv : storage.getTokensList()) {
392393
addToken(new Text(kv.getAliasBytes().toByteArray()),
393-
(Token<? extends TokenIdentifier>) new Token(kv.getToken()));
394+
ProtobufHelper.tokenFromProto(kv.getToken()));
394395
}
395396
for (CredentialsKVProto kv : storage.getSecretsList()) {
396397
addSecretKey(new Text(kv.getAliasBytes().toByteArray()),

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/token/Token.java

Lines changed: 0 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919
package org.apache.hadoop.security.token;
2020

2121
import com.google.common.collect.Maps;
22-
import com.google.protobuf.ByteString;
2322
import com.google.common.primitives.Bytes;
2423

2524
import org.apache.commons.codec.binary.Base64;
@@ -28,7 +27,6 @@
2827
import org.apache.hadoop.classification.InterfaceStability;
2928
import org.apache.hadoop.conf.Configuration;
3029
import org.apache.hadoop.io.*;
31-
import org.apache.hadoop.security.proto.SecurityProtos.TokenProto;
3230
import org.apache.hadoop.util.ReflectionUtils;
3331
import org.slf4j.Logger;
3432
import org.slf4j.LoggerFactory;
@@ -117,32 +115,6 @@ public Token<T> copyToken() {
117115
return new Token<T>(this);
118116
}
119117

120-
/**
121-
* Construct a Token from a TokenProto.
122-
* @param tokenPB the TokenProto object
123-
*/
124-
public Token(TokenProto tokenPB) {
125-
this.identifier = tokenPB.getIdentifier().toByteArray();
126-
this.password = tokenPB.getPassword().toByteArray();
127-
this.kind = new Text(tokenPB.getKindBytes().toByteArray());
128-
this.service = new Text(tokenPB.getServiceBytes().toByteArray());
129-
}
130-
131-
/**
132-
* Construct a TokenProto from this Token instance.
133-
* @return a new TokenProto object holding copies of data in this instance
134-
*/
135-
public TokenProto toTokenProto() {
136-
return TokenProto.newBuilder().
137-
setIdentifier(ByteString.copyFrom(this.getIdentifier())).
138-
setPassword(ByteString.copyFrom(this.getPassword())).
139-
setKindBytes(ByteString.copyFrom(
140-
this.getKind().getBytes(), 0, this.getKind().getLength())).
141-
setServiceBytes(ByteString.copyFrom(
142-
this.getService().getBytes(), 0, this.getService().getLength())).
143-
build();
144-
}
145-
146118
/**
147119
* Get the token identifier's byte representation.
148120
* @return the token identifier's byte representation

hadoop-hdfs-project/hadoop-hdfs-client/dev-support/findbugsExcludeFile.xml

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -92,10 +92,5 @@
9292
<Method name="getSymlinkInBytes" />
9393
<Bug pattern="EI_EXPOSE_REP" />
9494
</Match>
95-
<Match>
96-
<Class name="org.apache.hadoop.hdfs.protocolPB.PBHelperClient" />
97-
<Method name="getFixedByteString" />
98-
<Bug pattern="AT_OPERATION_SEQUENCE_ON_CONCURRENT_ABSTRACTION" />
99-
</Match>
10095

10196
</FindBugsFilter>

hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/protocolPB/PBHelperClient.java

Lines changed: 6 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@
2727
import java.util.List;
2828
import java.util.Map;
2929
import java.util.Set;
30-
import java.util.concurrent.ConcurrentHashMap;
3130

3231
import com.google.common.base.Preconditions;
3332
import com.google.common.cache.CacheBuilder;
@@ -207,6 +206,7 @@
207206
import org.apache.hadoop.io.EnumSetWritable;
208207
import org.apache.hadoop.io.Text;
209208
import org.apache.hadoop.io.erasurecode.ECSchema;
209+
import org.apache.hadoop.ipc.ProtobufHelper;
210210
import org.apache.hadoop.security.proto.SecurityProtos.TokenProto;
211211
import org.apache.hadoop.security.token.Token;
212212
import org.apache.hadoop.util.ChunkedArrayList;
@@ -232,33 +232,8 @@ public class PBHelperClient {
232232
private static final FsAction[] FSACTION_VALUES =
233233
FsAction.values();
234234

235-
/**
236-
* Map used to cache fixed strings to ByteStrings. Since there is no
237-
* automatic expiration policy, only use this for strings from a fixed, small
238-
* set.
239-
* <p/>
240-
* This map should not be accessed directly. Used the getFixedByteString
241-
* methods instead.
242-
*/
243-
private static ConcurrentHashMap<Object, ByteString> fixedByteStringCache =
244-
new ConcurrentHashMap<>();
245-
246-
private static ByteString getFixedByteString(Text key) {
247-
ByteString value = fixedByteStringCache.get(key);
248-
if (value == null) {
249-
value = ByteString.copyFromUtf8(key.toString());
250-
fixedByteStringCache.put(new Text(key.copyBytes()), value);
251-
}
252-
return value;
253-
}
254-
255235
private static ByteString getFixedByteString(String key) {
256-
ByteString value = fixedByteStringCache.get(key);
257-
if (value == null) {
258-
value = ByteString.copyFromUtf8(key);
259-
fixedByteStringCache.put(key, value);
260-
}
261-
return value;
236+
return ProtobufHelper.getFixedByteString(key);
262237
}
263238

264239
/**
@@ -281,7 +256,7 @@ private PBHelperClient() {
281256

282257
public static ByteString getByteString(byte[] bytes) {
283258
// return singleton to reduce object allocation
284-
return (bytes.length == 0) ? ByteString.EMPTY : ByteString.copyFrom(bytes);
259+
return ProtobufHelper.getByteString(bytes);
285260
}
286261

287262
public static ShmId convert(ShortCircuitShmIdProto shmId) {
@@ -349,12 +324,7 @@ public static ExtendedBlockProto convert(final ExtendedBlock b) {
349324
}
350325

351326
public static TokenProto convert(Token<?> tok) {
352-
TokenProto.Builder builder = TokenProto.newBuilder().
353-
setIdentifier(getByteString(tok.getIdentifier())).
354-
setPassword(getByteString(tok.getPassword())).
355-
setKindBytes(getFixedByteString(tok.getKind())).
356-
setServiceBytes(getFixedByteString(tok.getService()));
357-
return builder.build();
327+
return ProtobufHelper.protoFromToken(tok);
358328
}
359329

360330
public static ShortCircuitShmIdProto convert(ShmId shmId) {
@@ -832,11 +802,8 @@ public static StorageType[] convertStorageTypes(
832802

833803
public static Token<BlockTokenIdentifier> convert(
834804
TokenProto blockToken) {
835-
Token<BlockTokenIdentifier> token =
836-
new Token<>(blockToken.getIdentifier()
837-
.toByteArray(), blockToken.getPassword().toByteArray(), new Text(
838-
blockToken.getKind()), new Text(blockToken.getService()));
839-
return token;
805+
return (Token<BlockTokenIdentifier>) ProtobufHelper
806+
.tokenFromProto(blockToken);
840807
}
841808

842809
// DatanodeId

0 commit comments

Comments
 (0)