You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "chungen0126 (via GitHub)" <gi...@apache.org> on 2026/10/15 13:05:54 UTC
[PR] HDDS-15886. Use block length from datanode for under-construction block in StreamBlockInputStream [ozone]
chungen0126 opened a new pull request, #11551:
URL: https://github.com/apache/ozone/pull/11551
## What changes were proposed in this pull request?
With `ozone.client.stream.read.block` enabled, a reader can't read all hsync'ed data of a file that is still being written. It hits EOF at the block length OM last recorded, which can be stale for the last block because `hsyncKey` only updates OM when the last block changes. `BlockInputStream` handles this by getting the real length from the datanode for an under-construction block; `StreamBlockInputStream` did not.
This PR passes the under-construction flag to `StreamBlockInputStream`. Before the first read, the stream sends one `GetBlock` to get the real length, using the existing retry handling. Other blocks send no extra RPC.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-15886
## How was this patch tested?
- New unit tests in `TestStreamBlockInputStream`.
- `TestOzoneContractFSO` with `streamReadBlock` enabled: `testSyncable` fails before this change and passes after. It also passes with `streamReadBlock` disabled.
Generated-by: Claude Code (Claude Opus 5.5)
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
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-15886. Use block length from datanode for under-construction block in StreamBlockInputStream [ozone]
Posted by "ss77892 (via GitHub)" <gi...@apache.org>.
ss77892 commented on PR #11551:
URL: https://github.com/apache/ozone/pull/11551#issuecomment-6200189474
> @ss77892 PTAL
@chungen0126 Thank you for working on this! I wonder whether we need to call initialize at the top of MultipartInputStream.readPositioned() as well?
--
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-15886. Use block length from datanode for under-construction block in StreamBlockInputStream [ozone]
Posted by "chungen0126 (via GitHub)" <gi...@apache.org>.
chungen0126 commented on PR #11551:
URL: https://github.com/apache/ozone/pull/11551#issuecomment-6205784440
> > @ss77892 PTAL
> > @chungen0126 Thank you for working on this! I wonder whether we need to call initialize at the top of MultipartInputStream.readPositioned() as well?
Good point. `readPositioned()` previously stopped at the stale OM key length before `initialize()` was called, returning early EOF. Added `initialization`, and marked initializedas volatile to avoid lock contention on concurrent reads. Also added a unit test.
--
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-15886. Use block length from datanode for under-construction block in StreamBlockInputStream [ozone]
Posted by "chungen0126 (via GitHub)" <gi...@apache.org>.
chungen0126 commented on PR #11551:
URL: https://github.com/apache/ozone/pull/11551#issuecomment-6308200533
> @chungen0126 thanks for the patch! Could you add test in IT like `TestHSync` to cover the repro step that wasnt passed without this patch?
Add tests.
--
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-15886. Use block length from datanode for under-construction block in StreamBlockInputStream [ozone]
Posted by "chungen0126 (via GitHub)" <gi...@apache.org>.
chungen0126 commented on PR #11551:
URL: https://github.com/apache/ozone/pull/11551#issuecomment-6198683777
@ss77892 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-15886. Use block length from datanode for under-construction block in StreamBlockInputStream [ozone]
Posted by "peterxcli (via GitHub)" <gi...@apache.org>.
peterxcli commented on code in PR #11551:
URL: https://github.com/apache/ozone/pull/11551#discussion_r4332453578
##########
hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/storage/StreamBlockInputStream.java:
##########
@@ -105,8 +107,19 @@ public StreamBlockInputStream(
XceiverClientFactory xceiverClientFactory,
Function<BlockID, BlockLocationInfo> refreshFunction,
OzoneClientConfig config) throws IOException {
- this.blockID = blockID;
- this.blockLength = length;
+ this(new BlockLocationInfo.Builder().setBlockID(blockID).setLength(length).build(),
+ pipeline, token, xceiverClientFactory, refreshFunction, config);
+ }
+
+ public StreamBlockInputStream(
+ BlockLocationInfo blockInfo, Pipeline pipeline,
+ Token<OzoneBlockTokenIdentifier> token,
+ XceiverClientFactory xceiverClientFactory,
+ Function<BlockID, BlockLocationInfo> refreshFunction,
+ OzoneClientConfig config) throws IOException {
Review Comment:
let's remove old constructor and stick with this?
--
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-15886. Use block length from datanode for under-construction block in StreamBlockInputStream [ozone]
Posted by "peterxcli (via GitHub)" <gi...@apache.org>.
peterxcli commented on PR #11551:
URL: https://github.com/apache/ozone/pull/11551#issuecomment-6312324319
Thanks @chungen0126 for the patch, @ss77892, @szetszwo 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-15886. Use block length from datanode for under-construction block in StreamBlockInputStream [ozone]
Posted by "chungen0126 (via GitHub)" <gi...@apache.org>.
chungen0126 commented on PR #11551:
URL: https://github.com/apache/ozone/pull/11551#issuecomment-6198664869
@szetszwo 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-15886. Use block length from datanode for under-construction block in StreamBlockInputStream [ozone]
Posted by "peterxcli (via GitHub)" <gi...@apache.org>.
peterxcli commented on PR #11551:
URL: https://github.com/apache/ozone/pull/11551#issuecomment-6306886710
this might add many round trips when streaming read, we might want to add `blockLength` into `ReadBlockResponseProto`, and datanode always keep it up-to-date.
--
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-15886. Use block length from datanode for under-construction block in StreamBlockInputStream [ozone]
Posted by "chungen0126 (via GitHub)" <gi...@apache.org>.
chungen0126 commented on PR #11551:
URL: https://github.com/apache/ozone/pull/11551#issuecomment-6308157417
Thanks @peterxcli. Since those are mostly refactoring changes, I've created a follow-up JIRA ([HDDS-17206](https://issues.apache.org/jira/browse/HDDS-17206)) to address them together. Let's keep this PR focused on the bug fix itself.
--
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-15886. Use block length from datanode for under-construction block in StreamBlockInputStream [ozone]
Posted by "peterxcli (via GitHub)" <gi...@apache.org>.
peterxcli merged PR #11551:
URL: https://github.com/apache/ozone/pull/11551
--
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-15886. Use block length from datanode for under-construction block in StreamBlockInputStream [ozone]
Posted by "chungen0126 (via GitHub)" <gi...@apache.org>.
chungen0126 commented on code in PR #11551:
URL: https://github.com/apache/ozone/pull/11551#discussion_r4289934594
##########
hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/storage/StreamBlockInputStream.java:
##########
@@ -77,7 +77,9 @@ public class StreamBlockInputStream extends BlockExtendedInputStream {
private final String name = "stream" + STREAM_ID.getAndIncrement();
private final BlockID blockID;
- private final long blockLength;
+ private long blockLength;
+ // The block is under construction, so its length is fetched from the datanode before the first access.
+ private boolean underConstruction;
Review Comment:
Updated.
--
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-15886. Use block length from datanode for under-construction block in StreamBlockInputStream [ozone]
Posted by "chungen0126 (via GitHub)" <gi...@apache.org>.
chungen0126 commented on PR #11551:
URL: https://github.com/apache/ozone/pull/11551#issuecomment-6207079538
@ss77892 Would you mind taking another look?
--
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-15886. Use block length from datanode for under-construction block in StreamBlockInputStream [ozone]
Posted by "peterxcli (via GitHub)" <gi...@apache.org>.
peterxcli commented on code in PR #11551:
URL: https://github.com/apache/ozone/pull/11551#discussion_r4332509978
##########
hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/storage/StreamBlockInputStream.java:
##########
@@ -431,6 +446,27 @@ private XceiverClientGrpc acquireGrpcClientForReadData(XceiverClientFactory fact
return (XceiverClientGrpc) acquired;
}
+ /**
+ * Use the block length from the datanode if the block is under construction,
+ * since the length from OM may be stale (e.g. the last block of an hsync-ed file).
+ */
+ public synchronized void updateBlockLength() throws IOException {
+ while (needUpdateBlockLength) {
Review Comment:
why while? for retry? then should we set the retry limit?
##########
hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/storage/StreamBlockInputStream.java:
##########
@@ -431,6 +446,27 @@ private XceiverClientGrpc acquireGrpcClientForReadData(XceiverClientFactory fact
return (XceiverClientGrpc) acquired;
}
+ /**
+ * Use the block length from the datanode if the block is under construction,
+ * since the length from OM may be stale (e.g. the last block of an hsync-ed file).
+ */
+ public synchronized void updateBlockLength() throws IOException {
+ while (needUpdateBlockLength) {
+ try {
+ acquireClient();
+ blockLength = ContainerProtocolCalls.getBlock(xceiverClient, blockID, tokenRef.get(), pipelineRef.get())
+ .getBlockData().getSize();
Review Comment:
https://github.com/apache/ozone/blob/df42f7f60a2db3c06ed85e781ffdf89e02e0ccf0/hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/storage/BlockInputStream.java#L302-L313
https://github.com/apache/ozone/blob/df42f7f60a2db3c06ed85e781ffdf89e02e0ccf0/hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/storage/BlockInputStream.java#L228-L235
I saw there are lots of code inside `BlockInputStream` that already existed can be reused here, consider to put them into `BlockExtendedInputStream`?
--
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-15886. Use block length from datanode for under-construction block in StreamBlockInputStream [ozone]
Posted by "chungen0126 (via GitHub)" <gi...@apache.org>.
chungen0126 commented on PR #11551:
URL: https://github.com/apache/ozone/pull/11551#issuecomment-6205720120
> * Is it possible to refactor BlockInputStream and reuse the updateLength code?
The overlap is small: `BlockInputStream` needs the full BlockData anyway to build its chunk streams, while `StreamBlockInputStream` only needs the size.
--
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-15886. Use block length from datanode for under-construction block in StreamBlockInputStream [ozone]
Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on code in PR #11551:
URL: https://github.com/apache/ozone/pull/11551#discussion_r4287150366
##########
hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/storage/StreamBlockInputStream.java:
##########
@@ -77,7 +77,9 @@ public class StreamBlockInputStream extends BlockExtendedInputStream {
private final String name = "stream" + STREAM_ID.getAndIncrement();
private final BlockID blockID;
- private final long blockLength;
+ private long blockLength;
+ // The block is under construction, so its length is fetched from the datanode before the first access.
+ private boolean underConstruction;
Review Comment:
Let's call it `needUpdateBlockLength` since setting underConstruction to false is misleading (the block may be still underConstruction.)
--
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-15886. Use block length from datanode for under-construction block in StreamBlockInputStream [ozone]
Posted by "chungen0126 (via GitHub)" <gi...@apache.org>.
chungen0126 commented on code in PR #11551:
URL: https://github.com/apache/ozone/pull/11551#discussion_r4333091344
##########
hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/storage/StreamBlockInputStream.java:
##########
@@ -431,6 +446,27 @@ private XceiverClientGrpc acquireGrpcClientForReadData(XceiverClientFactory fact
return (XceiverClientGrpc) acquired;
}
+ /**
+ * Use the block length from the datanode if the block is under construction,
+ * since the length from OM may be stale (e.g. the last block of an hsync-ed file).
+ */
+ public synchronized void updateBlockLength() throws IOException {
+ while (needUpdateBlockLength) {
Review Comment:
The loop is for retries, and it is bounded: handleExceptions() calls shouldRetryRead() with the existing retryPolicy (same as initialize() for the streaming reader) and rethrows once the policy gives up. Non-retriable exceptions are rethrown immediately.
--
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-15886. Use block length from datanode for under-construction block in StreamBlockInputStream [ozone]
Posted by "chungen0126 (via GitHub)" <gi...@apache.org>.
chungen0126 commented on PR #11551:
URL: https://github.com/apache/ozone/pull/11551#issuecomment-6307693792
> this might add many round trips when streaming read, we might want to add `blockLength` into `ReadBlockResponseProto`, and datanode always keep it up-to-date.
Thanks @peterxcli for reviewing this. This adds at most one GetBlock per stream, and only for the last under-construction block.
Returning the length in ReadBlockResponseProto requires protocol changes and client EOF check adjustments. Even then, it only helps sequential reads—seeking directly beyond OM's stale length still inevitably requires an RPC.
Adding this much complexity just to avoid a single RPC isn't worth it. Let's this in a follow-up JIRA.
--
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