HDDS-16258. Refactor streaming block reads and fix checksum verification for variable-sized chunks by peterxcli · Pull Request #11302 · apache/ozone

peterxcli · 2026-09-22T17:15:46Z

What changes were proposed in this pull request?

Streaming ReadBlock collects checksums assuming uniform chunks, but flushes can produce short chunks whose checksum intervals restart at different block offsets.

This patch:

Move chunk lookup, checksum alignment, response sizing, and offset tracking from KeyValueHandler#readBlockImpl into BlockReadCursor. The handler consumes each range from the cursor, reads and sends the data, then advances the cursor. It no longer manages the boundary calculations itself.
Fix checksum verification across variable-sized chunks. Include the overlapping chunks’ original metadata and stored checksums in each response when requested, and verify the data using chunk-relative checksum boundaries on both the client and server.

The client sets includeChecksums from its existing verification setting. When false, the server omits chunkInfoList without changing response ranges or its own checksum verification. The request field defaults to true when omitted.

+--------------------------------------------------------------------------------------------------+
| SAME INPUT: full read [0,12), CRC32, bytesPerChecksum=4, response buffer >=12 bytes              |
|                                                                                                  |
| Chunk metadata: C1 offset=0, len=5; C2 offset=5, len=7                                           |
| Stored bytes:   [ A B C D E ] [ F G H I J K L ]                                                  |
| Stored CRCs:    C1: [ABCD] [E]    C2: [FGHI] [JKL]                                               |
| [ABCD] means the stored CRC of bytes ABCD; intervals restart in each chunk.                      |
+--------------------------------------------------------------------------------------------------+
                                                  |
                        +-------------------------+-------------------------+
                        v                                                   v
+----------------------------------------------+    +----------------------------------------------+
| BEFORE: SERVER / readBlockImpl               |    | AFTER: SERVER / readBlockImpl                |
| ReadBlockComputation + getChecksums()        |    | BlockReadCursor                              |
|                                              |    |                                              |
| Assumes every chunk is C1-sized (5 bytes).   |    | Selects [0,12) using chunk boundaries.       |
| Picks CRCs at block offsets 0, 4, 8:         |    | Includes the two overlapping chunks          |
| [ABCD] [E] [FGHI]  (drops [JKL])             |    | with their original offsets and CRCs.        |
+----------------------------------------------+    +----------------------------------------------+
                        |                                                   |
                        v                                                   v
+----------------------------------------------+    +----------------------------------------------+
| WIRE: ReadBlockResponseProto                 |    | WIRE: ReadBlockResponseProto                 |
| offset=0, data=ABCDEFGHIJKL                  |    | offset=0, data=ABCDEFGHIJKL                  |
| Flat checksumData:                           |    | chunkInfoList:                               |
|   [ABCD] [E] [FGHI]                          |    |   C1: offset=0, len=5, [ABCD] [E]            |
|                                              |    |   C2: offset=5, len=7, [FGHI] [JKL]          |
+----------------------------------------------+    +----------------------------------------------+
                        |                                                   |
                        v                                                   v
+----------------------------------------------+    +----------------------------------------------+
| BEFORE: CLIENT / StreamBlockInputStream      |    | AFTER: CLIENT / StreamBlockInputStream       |
| Splits the response into flat groups:        |    | Uses each chunk's checksum boundaries:       |
|   [ABCD] [EFGH] [IJKL]                       |    |   C1: [ABCD] [E]   C2: [FGHI] [JKL]          |
|                                              |    |                                              |
| CRC(EFGH) != stored CRC(E)                   |    | All stored CRCs match.                       |
| FAIL: response is not enqueued.              |    | PASS: enqueue ABCDEFGHIJKL.                  |
+----------------------------------------------+    +----------------------------------------------+

The production path now handles variable chunks without testVariableChunks or the legacy flattened checksum collector. Shared verification rejects missing coverage and invalid boundaries, handles NONE, and preserves buffer positions. Unexpected EOF fails before sending partial checksum data; OUT_OF_RANGE retains stream error delivery and file cleanup.

Protocol compatibility: ReadBlockResponseProto replaces the flattened checksum field with chunkInfoList = 4, reserves field 1 and checksumData, and preserves offset = 2 and data = 3. The release compatibility baseline (proto.lock) is unchanged. This intentionally requires matching clients and datanodes for streaming reads, which are disabled by default. There is no legacy fallback. Ordinary chunk reads and the stored format are unchanged.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-16258

How was this patch tested?

Validated on JDK 21 with regenerated protobufs, focused checksum/client/server/cursor tests, and checkstyle. Tests cover omitted metadata, unchanged response ranges, and independent datanode verification. Existing TestStreamRead integration coverage verifies uneven stored chunks through full and seek/range reads.

Copilot

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

Review effort: Lite
Findings: 1 · 2

Open (3)

This treats any short read as EOF and fails the request, but file/channel reads are not guaranteed… · New
Buffer sizing can grow to the maximum checksum interval seen in chunk metadata, which may cause… · New
A responseDataSize of 0 isn't rejected here, but BlockReadCursor rejects responseSize <= 0 by… · New

What changed in this PR

Updates streaming ReadBlock to correctly validate checksums across variable-length chunks by sending per-response overlapping ChunkInfo metadata and using a BlockReadCursor to align reads to chunk-relative checksum boundaries.

Changes:

Replace flattened checksum aggregation with BlockReadCursor + per-response chunkInfoList checksum validation.
Update wire protocol: reserve checksumData field and add chunkInfoList to ReadBlockResponseProto.
Expand unit/integration tests to cover uneven flushed chunks, unexpected EOF, and chunk-relative checksum verification.

File	Description
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/read/TestStreamRead.java	Adds integration coverage for uneven flushed chunks and range/seek verification with streaming reads.
hadoop-hdds/interface-client/src/main/proto/DatanodeClientProtocol.proto	Reworks ReadBlock response payload from flat checksum data to per-response chunk metadata.
hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueHandler.java	Updates tests for new handler signature and validates returned `chunkInfoList` + checksum verification behavior.
hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestBlockReadCursor.java	New unit tests for cursor range selection and boundary alignment.
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/ReadBlockComputation.java	Removes legacy checksum-aligned range computation helper.
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueHandler.java	Switches streaming readBlock implementation to `BlockReadCursor` and per-response chunk checksum validation.
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/BlockReadCursor.java	New cursor for chunk-relative checksum boundary alignment and response chunk selection.
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/interfaces/Handler.java	Updates handler interface to remove `testVariableChunks` plumbing.
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/HddsDispatcher.java	Adjusts dispatcher call site for new `readBlock` signature.
hadoop-hdds/common/src/test/java/org/apache/hadoop/ozone/common/TestChecksum.java	Adds tests for chunk-relative validation, malformed coverage/alignment, and NONE handling.
hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/common/Checksum.java	Reworks validation to operate on chunk-relative boundaries and detect missing/invalid coverage.
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/protocolPB/ContainerCommandResponseBuilders.java	Builds ReadBlock responses using `chunkInfoList` instead of flat checksum data.
hadoop-hdds/client/src/test/java/org/apache/hadoop/hdds/scm/storage/TestStreamBlockInputStream.java	Updates tests for new response format and adds coverage for seek-prefix trimming and missing metadata.
hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/storage/StreamBlockInputStream.java	Verifies each streaming response using `chunkInfoList` instead of `checksumData`.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

…limit

peterxcli · 2026-09-22T18:55:16Z

+  reserved 1;
+  reserved "checksumData";


I actually want to remove and renumber this as the streaming read is still off by default(means experimental), we could probably ignore the backward compatibility.

but that will make the check against proto.lock fail

With this change, an older client with streaming reads enabled would fail to parse responses after the datanodes are upgraded, even following a non-rolling cluster upgrade. The reverse, an upgraded client reading from older datanodes also fails when checksum verification is enabled because chunkInfoList is absent.

At least this is just a wire proto change and does not affect stored data. Users can disable streaming reads and reopen the client, or by bringing clients and datanodes onto compatible versions. Given that streaming reads are opt-in and this break is intentional for performance reason IIUC, I’m fine with the change for now, but could we document both failure scenarios and recovery steps in the release notes?

could we document both failure scenarios and recovery steps in the release notes?

yes, that's what I want.

chungen0126

Thanks @peterxcli for working on this.
Two suggestions:

Add a flag in the read request so the server skips sending chunkInfoList when verification isn't needed, saving payload size.
The server-side refactoring is out of scope here; please move it to a separate ticket.

chungen0126 · 2026-09-23T17:48:04Z

@@ -441,37 +439,39 @@ public static void verifyChecksum(List<ByteBuffer> bufferList, int startIndex, C
  public static void validateChecksums(ByteBuffer data, long blockOffset, int startIndex,


Adding those check is nice, but could you clarify the motivation behind refactoring this method? From my perspective, this change seems unnecessary. Please leave this part as it is.

peterxcli

The server-side refactoring is out of scope here; please move it to a separate ticket.

Sure, let me open and merge the server refactor as a separate PR first, then we'll get back to here.

Add a flag in the read request so the server skips sending chunkInfoList when verification isn't needed, saving payload size.

let me look into this, thanks for the suggestion

Streaming ReadBlock responses now list the chunks they cover (chunkInfoList) instead of carrying block-grid checksumData, so blocks with variable-size chunks, such as hsync-written files and blocks cut short at a container boundary, verify on both streaming read paths. Ported to the Ratis data stream read path: - KeyValueHandler reads with BlockReadCursor and still hands each response's data to the observer outside the protobuf. - RatisDataStreamBlockInputStream verifies replies with Checksum.validateChecksums over the chunk list. - ContainerStateMachine merges the chunk lists of consecutive responses, keeping a chunk that spans two responses once. - TestStreamRead#testUnevenFlushedChunks runs on both the gRPC and the Ratis read path and checks which block stream served it.

peterxcli · 2026-10-05T02:57:10Z

@ss77892 would you like to take a look?

peterxcli · 2026-10-05T15:22:47Z

@szetszwo would you like to take a look? thanks!

smengcl

Thanks @peterxcli for the patch.

smengcl · 2026-10-07T20:39:19Z

Thanks @peterxcli for the patch. Thanks @yandrey321 @chungen0126 @rich7420 for the reviews.

peterxcli · 2026-10-08T00:15:31Z

Thanks!

peterxcli · 2026-10-08T02:33:28Z

reminder for myself: remember to add release note about the compatibility issue on jira.

* master: (23 commits) HDDS-15951. Reduce ofs getFileChecksum to a single OM RPC (apache#11380) HDDS-16746. Local Stream onCommand should drain buffers before commit PutBlock (apache#11427) HDDS-16781. Fix intermittent ReadChunk count mismatch in TestKeyInputStream (apache#11443) HDDS-16779. FSO rename of a file to itself should be a no-op (apache#11440) HDDS-16724. Add support for missing S3 apis to STS (apache#11413) HDDS-16620. Expose BlockReadCursor as an iterator of streaming ReadBlock ranges (apache#11442) HDDS-16624. Improve AbortIncompleteMultipartUpload lifecycle validation error message (apache#11344) HDDS-16367. Update threat model with information about internal protocols (apache#11185) HDDS-16304. Convert RpcMetrics latency counters to lock-free ConcurrentMutableStat (apache#11425) HDDS-15681. Switch to io.github.ascopes:protobuf-maven-plugin (apache#11429) HDDS-16258. Refactor streaming block reads and fix checksum verification for variable-sized chunks (apache#11302) HDDS-16598. Fix intermittent failure in TestContainerStateMachine (apache#11319) HDDS-15802. Add integration tests for per-volume push replication (apache#11146) HDDS-16657. Reject lifecycle Transition actions in PutBucketLifecycleConfiguration (apache#11430) HDDS-16714. Split kubernetes check by environment (apache#11406) HDDS-16752. Bump guava to 33.7.2-jre (apache#11433) HDDS-16581. Bump ozone-runner to 20260922-1 (apache#11431) HDDS-16611. Use MutableGaugeLong for total count metrics in OMMetrics (apache#11410) HDDS-16738. Let dependabot ignore jetty minor version upgrades (apache#11422) HDDS-16683. Upgrade Jetty from 12.0.38 to 12.1.14 (apache#11424) ... Conflicts: hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneManagerVersion.java hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueHandler.java hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/ReplicationManager.java hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/TestReplicationManager.java hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileCreateRequestWithFSO.java

HDDS-16258. Verify streaming block reads using chunk boundaries

8bff57b

Copilot AI lite review requested due to automatic review settings September 22, 2026 17:15

HDDS-16258. Keep existing streaming response test builders separate

cfd49a2

Copilot AI reviewed Sep 22, 2026

View reviewed changes

peterxcli added 3 commits September 23, 2026 01:42

HDDS-16258. Simplify streaming response schema and range validation

c5f8eb6

HDDS-16258. Preserve the release protobuf compatibility baseline

31d0ffb

HDDS-16258. Bound streaming response buffers using the existing read …

cdae9ed

…limit

Copilot started reviewing on behalf of peterxcli September 22, 2026 18:18

View session

peterxcli added 3 commits September 23, 2026 02:35

HDDS-16258. Use chunk offsets for cursor boundary lookup

da3fb37

HDDS-16258. Clarify cursor boundary search

f0dfd43

HDDS-16258. Retain the streaming alignment constant

4dc5af7

peterxcli requested a balanced review from Copilot September 22, 2026 18:51

This comment was marked as resolved.

Sign in to view

peterxcli requested review from smengcl and szetszwo September 22, 2026 18:51

peterxcli marked this pull request as ready for review September 22, 2026 18:52

yandrey321 reviewed Sep 22, 2026

View reviewed changes

Comment thread ...tainer-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/BlockReadCursor.java

peterxcli commented Sep 22, 2026

View reviewed changes

szetszwo requested a review from chungen0126 September 22, 2026 19:41

github-actions Bot added client datanode labels Sep 23, 2026

chungen0126 reviewed Sep 23, 2026

View reviewed changes

peterxcli commented Sep 23, 2026

View reviewed changes

Comment thread hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/storage/StreamBlockInputStream.java

Comment thread ...tainer-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueHandler.java Outdated

HDDS-16258. Keep validation focused on streaming checksums

ece80ba

peterxcli changed the title ~~HDDS-16258. Verify streaming block reads using chunk boundaries~~ HDDS-16258. Refactor streaming block reads and fix checksum verification for variable-sized chunks Sep 27, 2026

peterxcli requested review from chungen0126 and yandrey321 September 27, 2026 13:02

peterxcli added 2 commits September 27, 2026 21:18

HDDS-16258. Omit streaming response checksums when not requested

6505008

HDDS-16258. Remove redundant includeChecksums comment

ca5728f

peterxcli removed the request for review from yandrey321 September 28, 2026 06:32

peterxcli requested a review from jojochuang September 28, 2026 06:32

Merge branch 'master' into HDDS-16258

cc04676

peterxcli mentioned this pull request Sep 30, 2026

HDDS-9904. Add Ratis-backed streaming reads #10743

Draft

HDDS-16258. Merge master and preserve streaming checksum verification

d3769cd

peterxcli mentioned this pull request Oct 6, 2026

HDDS-16722. Serve ReadBlock over Ratis read-only data streams on the datanode #11412

Open

smengcl approved these changes Oct 7, 2026

View reviewed changes

smengcl merged commit d319875 into apache:master Oct 7, 2026
44 checks passed

peterxcli deleted the HDDS-16258 branch October 8, 2026 00:15

peterxcli mentioned this pull request Oct 8, 2026

HDDS-16620. Expose BlockReadCursor as an iterator of streaming ReadBlock ranges #11442

Merged

		@@ -441,37 +439,39 @@ public static void verifyChecksum(List<ByteBuffer> bufferList, int startIndex, C
		public static void validateChecksums(ByteBuffer data, long blockOffset, int startIndex,

Conversation

peterxcli commented Sep 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

What is the link to the Apache JIRA

How was this patch tested?

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment was marked as resolved.

Uh oh!

Uh oh!

peterxcli Sep 22, 2026

Choose a reason for hiding this comment

Uh oh!

smengcl Sep 25, 2026

Choose a reason for hiding this comment

Uh oh!

peterxcli Sep 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chungen0126 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

chungen0126 Sep 23, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

peterxcli left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

peterxcli commented Oct 5, 2026

Uh oh!

peterxcli commented Oct 5, 2026

Uh oh!

smengcl left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

smengcl commented Oct 7, 2026

Uh oh!

peterxcli commented Oct 8, 2026

Uh oh!

peterxcli commented Oct 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

peterxcli commented Sep 22, 2026 •

edited

Loading

peterxcli Sep 25, 2026 •

edited

Loading