-
Notifications
You must be signed in to change notification settings - Fork 490
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
HDDS-4285. Read is slow due to frequent calls to UGI.getCurrentUser() and getTokens() #1454
Changes from 4 commits
86dbee8
32387fc
980842e
3fd7d07
e05eb00
5617fac
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,6 +20,7 @@ | |
|
||
import java.io.IOException; | ||
import java.util.ArrayList; | ||
import java.util.Collection; | ||
import java.util.HashMap; | ||
import java.util.List; | ||
import java.util.Map; | ||
|
@@ -61,6 +62,7 @@ | |
import org.apache.hadoop.security.UserGroupInformation; | ||
import org.apache.hadoop.security.token.Token; | ||
|
||
import org.apache.hadoop.security.token.TokenIdentifier; | ||
import org.apache.ratis.thirdparty.com.google.protobuf.ByteString; | ||
|
||
/** | ||
|
@@ -80,11 +82,14 @@ private ContainerProtocolCalls() { | |
* | ||
* @param xceiverClient client to perform call | ||
* @param datanodeBlockID blockID to identify container | ||
* @param tokens list of tokens the current user has, possibly including a | ||
* token for this block | ||
* @return container protocol get block response | ||
* @throws IOException if there is an I/O error while performing the call | ||
*/ | ||
public static GetBlockResponseProto getBlock(XceiverClientSpi xceiverClient, | ||
DatanodeBlockID datanodeBlockID) throws IOException { | ||
DatanodeBlockID datanodeBlockID, | ||
Collection<Token<? extends TokenIdentifier>> tokens) throws IOException { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we update the javadoc comment of this method as we new added param tokens here?
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks @linyiqun for the suggestion. Would @param tokens list of tokens the current user has, possibly including a token for this block be OK, or do you have a better description for this param? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @adoroszlai , above description looks good to me. Thanks. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can directly encode the pass in token for the specific block id without additional token selection. |
||
GetBlockRequestProto.Builder readBlockRequest = GetBlockRequestProto | ||
.newBuilder() | ||
.setBlockID(datanodeBlockID); | ||
|
@@ -96,7 +101,8 @@ public static GetBlockResponseProto getBlock(XceiverClientSpi xceiverClient, | |
.setContainerID(datanodeBlockID.getContainerID()) | ||
.setDatanodeUuid(id) | ||
.setGetBlock(readBlockRequest); | ||
String encodedToken = getEncodedBlockToken(getService(datanodeBlockID)); | ||
String encodedToken = getEncodedBlockToken(getService(datanodeBlockID), | ||
tokens); | ||
if (encodedToken != null) { | ||
builder.setEncodedToken(encodedToken); | ||
} | ||
|
@@ -181,13 +187,16 @@ public static ContainerProtos.PutBlockResponseProto putBlock( | |
* @param xceiverClient client to perform call | ||
* @param containerBlockData block data to identify container | ||
* @param eof whether this is the last putBlock for the same block | ||
* @param tokens list of tokens the current user has, possibly including a | ||
* token for this block | ||
* @return putBlockResponse | ||
* @throws IOException if there is an error while performing the call | ||
* @throws InterruptedException | ||
* @throws ExecutionException | ||
*/ | ||
public static XceiverClientReply putBlockAsync( | ||
XceiverClientSpi xceiverClient, BlockData containerBlockData, boolean eof) | ||
XceiverClientSpi xceiverClient, BlockData containerBlockData, boolean eof, | ||
Collection<Token<? extends TokenIdentifier>> tokens) | ||
throws IOException, InterruptedException, ExecutionException { | ||
PutBlockRequestProto.Builder createBlockRequest = | ||
PutBlockRequestProto.newBuilder() | ||
|
@@ -200,7 +209,8 @@ public static XceiverClientReply putBlockAsync( | |
.setDatanodeUuid(id) | ||
.setPutBlock(createBlockRequest); | ||
String encodedToken = | ||
getEncodedBlockToken(getService(containerBlockData.getBlockID())); | ||
getEncodedBlockToken(getService(containerBlockData.getBlockID()), | ||
tokens); | ||
if (encodedToken != null) { | ||
builder.setEncodedToken(encodedToken); | ||
} | ||
|
@@ -215,12 +225,15 @@ public static XceiverClientReply putBlockAsync( | |
* @param chunk information about chunk to read | ||
* @param blockID ID of the block | ||
* @param validators functions to validate the response | ||
* @param tokens list of tokens the current user has, possibly including a | ||
* token for this block | ||
* @return container protocol read chunk response | ||
* @throws IOException if there is an I/O error while performing the call | ||
*/ | ||
public static ContainerProtos.ReadChunkResponseProto readChunk( | ||
XceiverClientSpi xceiverClient, ChunkInfo chunk, BlockID blockID, | ||
List<CheckedBiFunction> validators) throws IOException { | ||
List<CheckedBiFunction> validators, | ||
Collection<Token<? extends TokenIdentifier>> tokens) throws IOException { | ||
ReadChunkRequestProto.Builder readChunkRequest = | ||
ReadChunkRequestProto.newBuilder() | ||
.setBlockID(blockID.getDatanodeBlockIDProtobuf()) | ||
|
@@ -231,7 +244,7 @@ public static ContainerProtos.ReadChunkResponseProto readChunk( | |
.setContainerID(blockID.getContainerID()) | ||
.setDatanodeUuid(id).setReadChunk(readChunkRequest); | ||
String encodedToken = getEncodedBlockToken(new Text(blockID. | ||
getContainerBlockID().toString())); | ||
getContainerBlockID().toString()), tokens); | ||
if (encodedToken != null) { | ||
builder.setEncodedToken(encodedToken); | ||
} | ||
|
@@ -281,11 +294,13 @@ public static void writeChunk(XceiverClientSpi xceiverClient, ChunkInfo chunk, | |
* @param chunk information about chunk to write | ||
* @param blockID ID of the block | ||
* @param data the data of the chunk to write | ||
* @param tokens list of tokens the current user has, possibly including a | ||
* token for this block | ||
* @throws IOException if there is an I/O error while performing the call | ||
*/ | ||
public static XceiverClientReply writeChunkAsync( | ||
XceiverClientSpi xceiverClient, ChunkInfo chunk, BlockID blockID, | ||
ByteString data) | ||
ByteString data, Collection<Token<? extends TokenIdentifier>> tokens) | ||
throws IOException, ExecutionException, InterruptedException { | ||
WriteChunkRequestProto.Builder writeChunkRequest = | ||
WriteChunkRequestProto.newBuilder() | ||
|
@@ -297,7 +312,7 @@ public static XceiverClientReply writeChunkAsync( | |
.setContainerID(blockID.getContainerID()) | ||
.setDatanodeUuid(id).setWriteChunk(writeChunkRequest); | ||
String encodedToken = getEncodedBlockToken(new Text(blockID. | ||
getContainerBlockID().toString())); | ||
getContainerBlockID().toString()), tokens); | ||
if (encodedToken != null) { | ||
builder.setEncodedToken(encodedToken); | ||
} | ||
|
@@ -540,8 +555,13 @@ public static void validateContainerResponse( | |
private static String getEncodedBlockToken(Text service) | ||
throws IOException { | ||
UserGroupInformation ugi = UserGroupInformation.getCurrentUser(); | ||
return getEncodedBlockToken(service, ugi.getTokens()); | ||
} | ||
|
||
private static String getEncodedBlockToken(Text service, | ||
Collection<Token<? extends TokenIdentifier>> tokens) throws IOException { | ||
Token<OzoneBlockTokenIdentifier> token = | ||
OzoneBlockTokenSelector.selectBlockToken(service, ugi.getTokens()); | ||
OzoneBlockTokenSelector.selectBlockToken(service, tokens); | ||
if (token != null) { | ||
return token.encodeToUrlString(); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we just pass down the token from the caller directly and bypass the UGI addToken/getTokens completely?