HDDS-16315. Skip MD5 recomputation in CopyObject when source ETag can be reused - #11136
Draft
peterxcli wants to merge 1 commit into
Draft
HDDS-16315. Skip MD5 recomputation in CopyObject when source ETag can be reused#11136peterxcli wants to merge 1 commit into
peterxcli wants to merge 1 commit into
Conversation
… be reused A whole-object CopyObject writes byte-identical content, and since HDDS-10403 the source key metadata already carries its content-based ETag. When that stored ETag is a plain (non-multipart) MD5, reuse it as the destination ETag instead of wrapping the source stream in a DigestInputStream and re-hashing every byte. Sources with no stored ETag, or with an aggregate "-N" ETag from an MPU-created key, keep the current digesting behavior so the destination still gets a proper content MD5. Applied to both the normal write path and the datastream write path. UploadPartCopy is unchanged.
Contributor
There was a problem hiding this comment.
Pull request overview
Optimizes the S3 Gateway CopyObject implementation to avoid re-reading and re-hashing the source object when the source key already has a reusable, plain (non-multipart) MD5 ETag in its metadata. This reduces CPU usage on the gateway while keeping the destination key metadata and CopyObjectResponse ETag consistent.
Changes:
- Reuse the source key’s plain MD5 ETag on
CopyObject(skippingDigestInputStream) when safe; otherwise preserve existing digesting behavior. - Apply the same reuse-or-digest logic to both the normal write path and the datastream write path.
- Add/extend unit tests to cover ETag reuse, multipart (
-N) ETag recompute, and missing-ETag recompute scenarios.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/ObjectEndpoint.java | Adds ETag reuse selection logic for CopyObject and threads a reusedETag through the copy/write path. |
| hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/ObjectEndpointStreaming.java | Extends the datastream copy path to optionally reuse an ETag without digesting. |
| hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestObjectPut.java | Adds tests for reuse vs recompute behavior and updates digest-reset exception test setup. |
| hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestUploadWithStream.java | Adds streaming-path tests for ETag reuse and multipart-source recomputation. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
1123
to
+1126
| copyLength = dest.copyFrom(src, getIOBufferSize(expectedLength)); | ||
| String md5Hash = DatatypeConverter.printHexBinary(src.getMessageDigest().digest()).toLowerCase(); | ||
| dest.getMetadata().put(OzoneConsts.ETAG, md5Hash); | ||
| String eTag = reusedETag != null ? reusedETag | ||
| : DatatypeConverter.printHexBinary(((DigestInputStream) src).getMessageDigest().digest()).toLowerCase(); | ||
| dest.getMetadata().put(OzoneConsts.ETAG, eTag); |
Comment on lines
198
to
202
| writeLen = writeGuard.copyFrom(body, bufferSize); | ||
| String eTag = DatatypeConverter.printHexBinary(body.getMessageDigest().digest()) | ||
| .toLowerCase(); | ||
| String eTag = reusedETag != null ? reusedETag | ||
| : DatatypeConverter.printHexBinary(((DigestInputStream) body).getMessageDigest().digest()).toLowerCase(); | ||
| perf.appendMetaLatencyNanos(metadataLatencyNs); | ||
| writeGuard.getMetadata().put(OzoneConsts.ETAG, eTag); |
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?
The S3 Gateway CopyObject path re-hashes every byte of the object during the copy:
ObjectEndpoint#copyObjectwraps the sourceOzoneInputStreamin ajava.security.DigestInputStream, and the computed MD5 becomes the destination key's ETag (inObjectEndpoint#copyfor the normal write path and inObjectEndpointStreaming#copyKeyWithStreamfor the datastream path).A whole-object copy produces byte-identical content, and since HDDS-10403 the source key's metadata already carries its content-based ETag. When that stored ETag is a plain (non-multipart) MD5, it is also the correct content MD5 for the destination, so the recomputation is pure wasted CPU.
This PR:
-Nmultipart suffix, skips theDigestInputStreamentirely and stores the source's ETag on the destination key. TheCopyObjectResponseETag is still read back from the re-fetched destination key metadata, so the response ETag and the stored ETag stay consistent.-NETag from an MPU-created source) keeps the current digesting behavior, so the destination still gets a proper content MD5. For MPU-sourced whole-object copies this matches AWS, which returns a plain MD5 for copies under 5GB.ObjectEndpoint#copy/S3ObjectWriteGuard#copyFromand the datastream pathObjectEndpointStreaming#copyKeyWithStream.Pure CPU optimization; no protocol change and no proto change. UploadPartCopy is untouched — part ETags are digests of the part's own bytes and are produced on a separate code path.
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-16315
How was this patch tested?
New and updated unit tests in the s3gateway module (
TestObjectPut: 51 tests,TestUploadWithStream: 6 tests,TestMultipartUploadWithCopy: 3 tests — all passing):TestObjectPut#testCopyObjectReusesSourceETagWithoutRehashing— plain-MD5 source: the source ETag is reused; the thread-local MD5 digest is mocked to throw ifupdateis called, so any re-hashing fails the test. Also asserts theCopyObjectResponseETag equals the ETag stored on the destination key.TestObjectPut#testCopyObjectRecomputesETagForMultipartSource—-Nsource ETag: destination gets a freshly computed content MD5.TestObjectPut#testCopyObjectComputesETagWhenSourceHasNoETag— source without an ETag: destination gets a freshly computed content MD5.TestObjectPut#testCopyObjectMessageDigestResetDuringException— updated to seed the source with a-NETag so it keeps exercising the digesting path and its thread-local digest reset.TestUploadWithStream#testUploadWithCopyReusesSourceETagand#testUploadWithCopyRecomputesETagForMultipartSource— the same matrix for the datastream write path; the existing#testUploadWithCopycovers the streaming no-ETag case.TestMultipartUploadWithCopypasses unchanged, confirming UploadPartCopy behavior is unaffected../hadoop-ozone/dev-support/checks/checkstyle.shwas run over the repository and reports 0 violations.Benchmark
Measured on an Apple M4 / JDK 21 with an off-tree harness driving
ObjectEndpointagainst thein-memory client stub. The A/B is driven by the shipped code rather than by two builds: a source
whose stored ETag is a plain MD5 takes the new path, a source whose ETag ends in
-Ntakes thedigesting path. Arms are interleaved in one JVM, medians over >= 12 iterations after warmup.
Path selection was confirmed by injecting a counting
MessageDigestintoEndpointBase.getMD5DigestInstance(): the digesting arm feeds exactly 67,108,864 bytes to MD5 fora 64 MiB object (16 reads at the 4 MiB
ozone.s3g.client.buffer.size), the reuse arm feeds 0.The stub holds everything in memory, so these percentages are an upper bound and not end-to-end
cluster numbers. The portable result is the absolute one: the change removes roughly 1.2-1.4 s
of S3 Gateway CPU per GiB copied (JDK MD5 measures 0.72-0.81 GiB/s here;
openssl speed md5independently reports 0.85 GiB/s). Because MD5 is serial and runs on the request thread, that is
also ~1.2 core-seconds freed per GiB of copy throughput on a loaded gateway.
Generated-by: Claude Code (claude-fable-5)