You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "symious (via GitHub)" <gi...@apache.org> on 2026/09/15 02:23:26 UTC
[PR] HDDS-16419. Avoid nested datanode failover in GetBlock and ReadChunk [ozone]
symious opened a new pull request, #11239:
URL: https://github.com/apache/ozone/pull/11239
## What changes were proposed in this pull request?
`ContainerProtocolCalls#getBlock` and `#readChunk` fail over between datanodes
via `tryEachDatanode`. For each datanode, they call
`XceiverClientSpi#sendCommand(request, validators)`. In `XceiverClientGrpc` this
goes to `sendCommandWithRetry`, which again tries every datanode of the
pipeline. As a result:
- every datanode is contacted N times, so a single call can take
`N × N × ozone.client.read.timeout` before failing, and a read can take
`read.max.retries × N × N × read.timeout`;
- the request built for datanode A (`datanodeUuid`, EC replica index) may be
sent to datanode B.
This PR makes `tryEachDatanode` the only failover layer for these two commands:
- `XceiverClientSpi`
- Add `sendCommand(request, validators, datanode)`, which sends the command
to the given datanode only. The default implementation falls back to
`sendCommand(request, validators)`, so Ratis and short-circuit clients are
unchanged.
- Add `getDatanodesInOrder(blockID, cmdType)`. The default implementation
returns the datanodes in `Pipeline#getClosestNode` order, which is the
current behavior of `tryEachDatanode`.
- `XceiverClientGrpc`
- Extract the single-datanode part of `sendCommandWithRetry` into
`sendCommandToDatanode`: send, run validators, update the GetBlock
datanode cache. Both the existing retry loop and the new
`sendCommand(request, validators, datanode)` use it.
- `getDatanodesInOrder` reuses `sortDatanodes`, so the datanode order
(leader / cached GetBlock datanode first, non-IN_SERVICE last,
topology-aware or shuffled) is the same as before.
- `ContainerProtocolCalls`
- `tryEachDatanode` iterates over `getDatanodesInOrder`.
- The per-datanode GetBlock/ReadChunk calls use the new single-datanode
`sendCommand`.
Other commands (`listBlock`, `readSmallFile`, ...) still use the inner failover
and are not affected.
After this change, the worst case for a read is
`read.max.retries × N × read.timeout`.
Note: `getBlockFromDatanode` (used by `ozone debug replicas chunk-info`) now
really 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?
- `TestXceiverClientGrpc`
- `testGetBlockRetryAlNodes` / `testReadChunkRetryAllNodes` now assert that
each datanode is contacted exactly once (previously N² calls), and that
the request's `datanodeUuid` matches the datanode it is sent to.
- New `testReadChunkRetryNextNodeOnShortRead`: a short read from the first
datanode is retried on the next datanode.
- Existing ordering tests (leader, topology, IN_SERVICE first, connection
reuse after GetBlock) pass unchanged.
- `TestChunkInputStream`, `TestBlockInputStream` pass.
- Manually reproduced on a test cluster: with read.timeout=5s and 2
unresponsive datanodes, the read failed after 60s before this change.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-16419. Avoid nested datanode failover in GetBlock and ReadChunk [ozone]
Posted by "symious (via GitHub) via issues" <is...@ozone.apache.org>.
symious commented on code in PR #11239:
URL: https://github.com/apache/ozone/pull/11239#discussion_r4438314465
##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/XceiverClientSpi.java:
##########
@@ -159,6 +162,45 @@ public ContainerCommandResponseProto sendCommand(
}
}
+ /**
+ * Sends a given command to the given datanode only, without failing over
+ * to the other datanodes in the pipeline.
+ * Implementations which cannot target a specific datanode fall back to
+ * {@link #sendCommand(ContainerCommandRequestProto, List)}.
+ * @param request Request
+ * @param validators functions to validate the response
+ * @param datanode the datanode to send the command to
+ * @return Response to the command
+ */
+ public ContainerCommandResponseProto sendCommand(
+ ContainerCommandRequestProto request,
+ List<Validator> validators,
+ DatanodeDetails datanode)
+ throws IOException {
+ return sendCommand(request, validators);
+ }
+
+ /**
+ * Returns the datanodes of the pipeline in the order they should be tried
+ * for a command on the given block.
+ * @param blockID the block the command operates on
+ * @param cmdType type of the command
+ * @return datanodes of the pipeline, in the order to try
+ */
+ public List<DatanodeDetails> getDatanodesInOrder(
+ ContainerProtos.DatanodeBlockID blockID, ContainerProtos.Type cmdType)
+ throws IOException {
+ final Pipeline pipeline = getPipeline();
+ final List<DatanodeDetails> datanodes = new ArrayList<>(pipeline.size());
+ final Set<DatanodeDetails> excluded = new HashSet<>();
+ while (excluded.size() < pipeline.size()) {
+ final DatanodeDetails d = pipeline.getClosestNode(excluded);
+ datanodes.add(d);
+ excluded.add(d);
+ }
+ return datanodes;
Review Comment:
Updated, PTAL.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-16419. Avoid nested datanode failover in GetBlock and ReadChunk [ozone]
Posted by "symious (via GitHub)" <gi...@apache.org>.
symious commented on code in PR #11239:
URL: https://github.com/apache/ozone/pull/11239#discussion_r4152767033
##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/storage/ContainerProtocolCalls.java:
##########
@@ -197,7 +201,8 @@ public static GetBlockResponseProto getBlock(XceiverClientSpi xceiverClient,
builder.setEncodedToken(token.encodeToUrlString());
}
- return tryEachDatanode(xceiverClient.getPipeline(),
+ return tryEachDatanode(
+ xceiverClient.getDatanodesInOrder(blockID.getDatanodeBlockIDProtobuf(), Type.GetBlock),
Review Comment:
@rich7420 Thank you for the review, updated, PTAL.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-16419. Avoid nested datanode failover in GetBlock and ReadChunk [ozone]
Posted by "ChenSammi (via GitHub) via issues" <is...@ozone.apache.org>.
ChenSammi merged PR #11239:
URL: https://github.com/apache/ozone/pull/11239
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-16419. Avoid nested datanode failover in GetBlock and ReadChunk [ozone]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #11239:
URL: https://github.com/apache/ozone/pull/11239#issuecomment-6354107862
This PR has been marked as stale due to 21 days of inactivity. Please comment or remove the stale label to keep it open. Otherwise, it will be automatically closed in 7 days.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-16419. Avoid nested datanode failover in GetBlock and ReadChunk [ozone]
Posted by "symious (via GitHub)" <gi...@apache.org>.
symious commented on code in PR #11239:
URL: https://github.com/apache/ozone/pull/11239#discussion_r4375047437
##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/storage/ContainerProtocolCalls.java:
##########
@@ -256,7 +261,7 @@ private static GetBlockResponseProto getBlock(XceiverClientSpi xceiverClient,
.setDatanodeUuid(datanode.getUuidString())
.setGetBlock(readBlockRequest).build();
ContainerCommandResponseProto response =
- xceiverClient.sendCommand(request, validators);
+ xceiverClient.sendCommand(request, validators, datanode);
Review Comment:
Updated, PTAL.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-16419. Avoid nested datanode failover in GetBlock and ReadChunk [ozone]
Posted by "ChenSammi (via GitHub)" <gi...@apache.org>.
ChenSammi commented on code in PR #11239:
URL: https://github.com/apache/ozone/pull/11239#discussion_r4410712435
##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/XceiverClientSpi.java:
##########
@@ -159,6 +162,45 @@ public ContainerCommandResponseProto sendCommand(
}
}
+ /**
+ * Sends a given command to the given datanode only, without failing over
+ * to the other datanodes in the pipeline.
+ * Implementations which cannot target a specific datanode fall back to
+ * {@link #sendCommand(ContainerCommandRequestProto, List)}.
+ * @param request Request
+ * @param validators functions to validate the response
+ * @param datanode the datanode to send the command to
+ * @return Response to the command
+ */
+ public ContainerCommandResponseProto sendCommand(
+ ContainerCommandRequestProto request,
+ List<Validator> validators,
+ DatanodeDetails datanode)
+ throws IOException {
+ return sendCommand(request, validators);
+ }
+
+ /**
+ * Returns the datanodes of the pipeline in the order they should be tried
+ * for a command on the given block.
+ * @param blockID the block the command operates on
+ * @param cmdType type of the command
+ * @return datanodes of the pipeline, in the order to try
+ */
+ public List<DatanodeDetails> getDatanodesInOrder(
+ ContainerProtos.DatanodeBlockID blockID, ContainerProtos.Type cmdType)
+ throws IOException {
+ final Pipeline pipeline = getPipeline();
+ final List<DatanodeDetails> datanodes = new ArrayList<>(pipeline.size());
+ final Set<DatanodeDetails> excluded = new HashSet<>();
+ while (excluded.size() < pipeline.size()) {
+ final DatanodeDetails d = pipeline.getClosestNode(excluded);
+ datanodes.add(d);
+ excluded.add(d);
+ }
+ return datanodes;
Review Comment:
Thanks @symious , overall looks good to me.
I think for this function, we can just return getPipeline().getNodesInOrder().
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-16419. Avoid nested datanode failover in GetBlock and ReadChunk [ozone]
Posted by "ChenSammi (via GitHub)" <gi...@apache.org>.
ChenSammi commented on code in PR #11239:
URL: https://github.com/apache/ozone/pull/11239#discussion_r4410714452
##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/XceiverClientSpi.java:
##########
@@ -159,6 +162,45 @@ public ContainerCommandResponseProto sendCommand(
}
}
+ /**
+ * Sends a given command to the given datanode only, without failing over
+ * to the other datanodes in the pipeline.
+ * Implementations which cannot target a specific datanode fall back to
+ * {@link #sendCommand(ContainerCommandRequestProto, List)}.
+ * @param request Request
+ * @param validators functions to validate the response
+ * @param datanode the datanode to send the command to
+ * @return Response to the command
+ */
+ public ContainerCommandResponseProto sendCommand(
Review Comment:
Can we rename this to sendCommandToDatanode?
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-16419. Avoid nested datanode failover in GetBlock and ReadChunk [ozone]
Posted by "ChenSammi (via GitHub) via issues" <is...@ozone.apache.org>.
ChenSammi commented on PR #11239:
URL: https://github.com/apache/ozone/pull/11239#issuecomment-6575709287
Thanks @symious , and @yandrey321 @rich7420 @adoroszlai for the review.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-16419. Avoid nested datanode failover in GetBlock and ReadChunk [ozone]
Posted by "yandrey321 (via GitHub)" <gi...@apache.org>.
yandrey321 commented on PR #11239:
URL: https://github.com/apache/ozone/pull/11239#issuecomment-5673978803
lgtm
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-16419. Avoid nested datanode failover in GetBlock and ReadChunk [ozone]
Posted by "rich7420 (via GitHub)" <gi...@apache.org>.
rich7420 commented on code in PR #11239:
URL: https://github.com/apache/ozone/pull/11239#discussion_r4366129319
##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/storage/ContainerProtocolCalls.java:
##########
@@ -256,7 +261,7 @@ private static GetBlockResponseProto getBlock(XceiverClientSpi xceiverClient,
.setDatanodeUuid(datanode.getUuidString())
.setGetBlock(readBlockRequest).build();
ContainerCommandResponseProto response =
- xceiverClient.sendCommand(request, validators);
+ xceiverClient.sendCommand(request, validators, datanode);
Review Comment:
Please update `TestStreamBlockInputStream`: stub `getDatanodesInOrder`, and migrate `sendCommand` stubbing and both verifications to three arguments. [CI failure](https://github.com/apache/ozone/actions/runs/39921073365/job/120314111766).
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-16419. Avoid nested datanode failover in GetBlock and ReadChunk [ozone]
Posted by "ChenSammi (via GitHub)" <gi...@apache.org>.
ChenSammi commented on code in PR #11239:
URL: https://github.com/apache/ozone/pull/11239#discussion_r4410714452
##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/XceiverClientSpi.java:
##########
@@ -159,6 +162,45 @@ public ContainerCommandResponseProto sendCommand(
}
}
+ /**
+ * Sends a given command to the given datanode only, without failing over
+ * to the other datanodes in the pipeline.
+ * Implementations which cannot target a specific datanode fall back to
+ * {@link #sendCommand(ContainerCommandRequestProto, List)}.
+ * @param request Request
+ * @param validators functions to validate the response
+ * @param datanode the datanode to send the command to
+ * @return Response to the command
+ */
+ public ContainerCommandResponseProto sendCommand(
Review Comment:
Can we rename this to sendCommandToDatanode?
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-16419. Avoid nested datanode failover in GetBlock and ReadChunk [ozone]
Posted by "ChenSammi (via GitHub)" <gi...@apache.org>.
ChenSammi commented on code in PR #11239:
URL: https://github.com/apache/ozone/pull/11239#discussion_r4410712435
##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/XceiverClientSpi.java:
##########
@@ -159,6 +162,45 @@ public ContainerCommandResponseProto sendCommand(
}
}
+ /**
+ * Sends a given command to the given datanode only, without failing over
+ * to the other datanodes in the pipeline.
+ * Implementations which cannot target a specific datanode fall back to
+ * {@link #sendCommand(ContainerCommandRequestProto, List)}.
+ * @param request Request
+ * @param validators functions to validate the response
+ * @param datanode the datanode to send the command to
+ * @return Response to the command
+ */
+ public ContainerCommandResponseProto sendCommand(
+ ContainerCommandRequestProto request,
+ List<Validator> validators,
+ DatanodeDetails datanode)
+ throws IOException {
+ return sendCommand(request, validators);
+ }
+
+ /**
+ * Returns the datanodes of the pipeline in the order they should be tried
+ * for a command on the given block.
+ * @param blockID the block the command operates on
+ * @param cmdType type of the command
+ * @return datanodes of the pipeline, in the order to try
+ */
+ public List<DatanodeDetails> getDatanodesInOrder(
+ ContainerProtos.DatanodeBlockID blockID, ContainerProtos.Type cmdType)
+ throws IOException {
+ final Pipeline pipeline = getPipeline();
+ final List<DatanodeDetails> datanodes = new ArrayList<>(pipeline.size());
+ final Set<DatanodeDetails> excluded = new HashSet<>();
+ while (excluded.size() < pipeline.size()) {
+ final DatanodeDetails d = pipeline.getClosestNode(excluded);
+ datanodes.add(d);
+ excluded.add(d);
+ }
+ return datanodes;
Review Comment:
We can just return getPipeline().getNodesInOrder() for this function.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-16419. Avoid nested datanode failover in GetBlock and ReadChunk [ozone]
Posted by "rich7420 (via GitHub)" <gi...@apache.org>.
rich7420 commented on code in PR #11239:
URL: https://github.com/apache/ozone/pull/11239#discussion_r4143673273
##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/storage/ContainerProtocolCalls.java:
##########
@@ -197,7 +201,8 @@ public static GetBlockResponseProto getBlock(XceiverClientSpi xceiverClient,
builder.setEncodedToken(token.encodeToUrlString());
}
- return tryEachDatanode(xceiverClient.getPipeline(),
+ return tryEachDatanode(
+ xceiverClient.getDatanodesInOrder(blockID.getDatanodeBlockIDProtobuf(), Type.GetBlock),
Review Comment:
For RATIS, this lookup omits `replicaIndex`, but the cached request sets it to `0`, so the cache misses. With topology-aware reads disabled, my probe shows the next GetBlock retrying the failed leader instead of using the cached follower. Please align the cache keys and add a regression test for this case.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-16419. Avoid nested datanode failover in GetBlock and ReadChunk [ozone]
Posted by "rich7420 (via GitHub)" <gi...@apache.org>.
rich7420 commented on PR #11239:
URL: https://github.com/apache/ozone/pull/11239#issuecomment-5909566184
@symious thanks for the patch!
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org