jojochuang commented on code in PR #4357:
URL: https://github.com/apache/ozone/pull/4357#discussion_r1132902545
##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/storage/ContainerProtocolCalls.java:
##########
@@ -687,12 +713,28 @@ public static void validateContainerResponse(
}
public static List<CheckedBiFunction> getValidatorList() {
- List<CheckedBiFunction> validators = new ArrayList<>(1);
+ return VALIDATORS;
+ }
+
+ static final List<CheckedBiFunction> VALIDATORS = createValidatorList();
Review Comment:
private?
##########
hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/storage/BlockInputStream.java:
##########
@@ -249,14 +249,31 @@ protected List<ChunkInfo> getChunkInfos() throws
IOException {
blkIDBuilder.setReplicaIndex(replicaIndex);
}
GetBlockResponseProto response = ContainerProtocolCalls
- .getBlock(xceiverClient, blkIDBuilder.build(), token);
+ .getBlock(xceiverClient, VALIDATORS, blkIDBuilder.build(), token);
- chunks = response.getBlockData().getChunksList();
+ return response.getBlockData().getChunksList();
} finally {
releaseClient();
}
+ }
+
+ static final List<CheckedBiFunction> VALIDATORS = ContainerProtocolCalls
Review Comment:
private?
##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/storage/ContainerProtocolCalls.java:
##########
@@ -128,36 +131,72 @@ public static ListBlockResponseProto
listBlock(XceiverClientSpi xceiverClient,
return response.getListBlock();
}
+ static <T> T tryEachDatanode(Pipeline pipeline,
+ CheckedFunction<DatanodeDetails, T, IOException> op,
+ BiConsumer<DatanodeDetails, IOException> exceptionHandler)
+ throws IOException {
+ final Set<DatanodeDetails> excluded = new HashSet<>();
+ for (; ;) {
+ final DatanodeDetails d = pipeline.getClosestNode(excluded);
+
+ try {
+ return op.apply(d);
+ } catch (IOException e) {
+ excluded.add(d);
+ if (excluded.size() < pipeline.size()) {
+ exceptionHandler.accept(d, e);
+ } else {
+ throw e;
+ }
+ }
+ }
+ }
+
/**
* Calls the container protocol to get a container block.
*
* @param xceiverClient client to perform call
+ * @param validators functions to validate the response
* @param datanodeBlockID blockID to identify container
* @param token a token for this block (may be null)
* @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,
+ List<CheckedBiFunction> validators,
DatanodeBlockID datanodeBlockID,
Token<? extends TokenIdentifier> token) throws IOException {
GetBlockRequestProto.Builder readBlockRequest = GetBlockRequestProto
.newBuilder()
.setBlockID(datanodeBlockID);
- String id = xceiverClient.getPipeline().getFirstNode().getUuidString();
-
ContainerCommandRequestProto.Builder builder = ContainerCommandRequestProto
.newBuilder()
.setCmdType(Type.GetBlock)
.setContainerID(datanodeBlockID.getContainerID())
- .setDatanodeUuid(id)
.setGetBlock(readBlockRequest);
if (token != null) {
builder.setEncodedToken(token.encodeToUrlString());
}
- ContainerCommandRequestProto request = builder.build();
+ return tryEachDatanode(xceiverClient.getPipeline(),
+ d -> getBlock(xceiverClient, validators, builder, d),
+ (d, e) -> LOG.warn("Failed to getBlock from " + d, e));
Review Comment:
My understanding is that client will log the warning for the first and
second failure, but the third failure it will simply throw the exception. I
think it would be clear if the message is something like "Failed to get Block
from " + d + ", will try another DataNode"
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
