Conversation
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
|
lgtm |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
ContainerProtocolCalls#getBlockand#readChunkfail over between datanodesvia
tryEachDatanode. For each datanode, they callXceiverClientSpi#sendCommand(request, validators). InXceiverClientGrpcthisgoes to
sendCommandWithRetry, which again tries every datanode of thepipeline. As a result:
N × N × ozone.client.read.timeoutbefore failing, and a read can takeread.max.retries × N × N × read.timeout;datanodeUuid, EC replica index) may besent to datanode B.
This PR makes
tryEachDatanodethe only failover layer for these two commands:XceiverClientSpisendCommand(request, validators, datanode), which sends the commandto the given datanode only. The default implementation falls back to
sendCommand(request, validators), so Ratis and short-circuit clients areunchanged.
getDatanodesInOrder(blockID, cmdType). The default implementationreturns the datanodes in
Pipeline#getClosestNodeorder, which is thecurrent behavior of
tryEachDatanode.XceiverClientGrpcsendCommandWithRetryintosendCommandToDatanode: send, run validators, update the GetBlockdatanode cache. Both the existing retry loop and the new
sendCommand(request, validators, datanode)use it.getDatanodesInOrderreusessortDatanodes, so the datanode order(leader / cached GetBlock datanode first, non-IN_SERVICE last,
topology-aware or shuffled) is the same as before.
ContainerProtocolCallstryEachDatanodeiterates overgetDatanodesInOrder.sendCommand.Other commands (
listBlock,readSmallFile, ...) still use the inner failoverand are not affected.
After this change, the worst case for a read is
read.max.retries × N × read.timeout.Note:
getBlockFromDatanode(used byozone debug replicas chunk-info) nowreally queries only the given datanode. Before this change it could silently
return the result from another datanode.
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-16419
How was this patch tested?
TestXceiverClientGrpctestGetBlockRetryAlNodes/testReadChunkRetryAllNodesnow assert thateach datanode is contacted exactly once (previously N² calls), and that
the request's
datanodeUuidmatches the datanode it is sent to.testReadChunkRetryNextNodeOnShortRead: a short read from the firstdatanode is retried on the next datanode.
reuse after GetBlock) pass unchanged.
TestChunkInputStream,TestBlockInputStreampass.