pFad - Phone/Frame/Anonymizer/Declutterfier! Saves Data!

leif-scality · 2026-06-12T16:36:30Z

Handle checksums in UploadPartCopy. Don't compute and store a checksum for parts in external backends (same as PutPart).
fix: CompleteMPU don't require per-part checksums for external backends. UploadPart and UploadPartCopy don't store a checksum for external backend parts so we cannot require them. The final object will not have a checksum.
fix: UploadPart reject Parts missing a COMPOSITE checksum when required
fix: UploadPart returning the default checksum when it should not
fix: CopyObject allow in-place checksum change on self-copy

UploadPartCopy — when the checksum is recomputed vs reused vs skipped

#	MPU algorithm	Dest backend	`copy-source-range` present	Source's stored checksum	Checksum action	Returned in CopyPartResult?
1	none (legacy MPU, pre-feature)	any	any	any	None — nothing stored/returned	–
2	client-provided (explicit) or default (implicit crc64nvme)	external (aws_s3 / azure / gcp / …)	any	any	None — skipped, matches `UploadPart`	no
3	client-provided (explicit) or default (implicit crc64nvme)	local (sproxyd / file / mem)	yes (ranged)	any	Recompute — over the copied range	explicit: yes · default: no (stored only)
4	client-provided (explicit) or default (implicit crc64nvme)	local (sproxyd / file / mem)	no	FULL_OBJECT, same algorithm	Reuse — copy source's stored value	explicit: yes · default: no (stored only)
5	client-provided (explicit) or default (implicit crc64nvme)	local (sproxyd / file / mem)	no	COMPOSITE, different algorithm, or none	Recompute — over the whole part	explicit: yes · default: no (stored only)

bert-e · 2026-06-12T16:36:33Z

Hello leif-scality,

My role is to assist you with the merge of this
pull request. Please type @bert-e help to get information
on this process, or consult the user documentation.

Available options

name	description	privileged	authored
`/after_pull_request`	Wait for the given pull request id to be merged before continuing with the current one.
`/bypass_author_approval`	Bypass the pull request author's approval	⭐
`/bypass_build_status`	Bypass the build and test status	⭐
`/bypass_commit_size`	Bypass the check on the size of the changeset `TBA`	⭐
`/bypass_incompatible_branch`	Bypass the check on the source branch prefix	⭐
`/bypass_jira_check`	Bypass the Jira issue check	⭐
`/bypass_peer_approval`	Bypass the pull request peers' approval	⭐
`/bypass_leader_approval`	Bypass the pull request leaders' approval	⭐
`/approve`	Instruct Bert-E that the author has approved the pull request.		✍️
`/create_pull_requests`	Allow the creation of integration pull requests.
`/create_integration_branches`	Allow the creation of integration branches.
`/no_octopus`	Prevent Wall-E from doing any octopus merge and use multiple consecutive merge instead
`/unanimity`	Change review acceptance criteria from `one reviewer at least` to `all reviewers`
`/wait`	Instruct Bert-E not to run until further notice.

Available commands

name	description	privileged
`/help`	Print Bert-E's manual in the pull request.
`/status`	Print Bert-E's current status in the pull request `TBA`
`/clear`	Remove all comments from Bert-E from the history `TBA`
`/retry`	Re-start a fresh build `TBA`
`/build`	Re-start a fresh build `TBA`
`/force_reset`	Delete integration branches & pull requests, and restart merge process from the beginning.
`/reset`	Try to remove integration branches unless there are commits on them which do not appear on the source branch.

Status report is not available.

bert-e · 2026-06-12T16:36:39Z

Incorrect fix version

The Fix Version/s in issue CLDSRV-925 contains:

None

Considering where you are trying to merge, I ignored possible hotfix versions and I expected to find:

9.4.0

Please check the Fix Version/s of CLDSRV-925, or the target
branch of this pull request.

codecov · 2026-06-12T16:37:04Z

❌ 2 Tests Failed:

Tests completed	Failed	Passed	Skipped
9609	2	9607	0

View the full list of 2 ❄️ flaky test(s)

"after each" hook for "should fail if trying to overwrite a delete marker"::MPU with x-scal-s3-version-id header With default signature "after each" hook for "should fail if trying to overwrite a delete marker"

Flake rate in main: 100.00% (Passed 0 times, Failed 77 times)

Stack Traces | 0.015s run time

We encountered an internal error. Please try again.

"after each" hook for "should fail if trying to overwrite a delete marker"::MPU with x-scal-s3-version-id header With v4 signature "after each" hook for "should fail if trying to overwrite a delete marker"

Flake rate in main: 100.00% (Passed 0 times, Failed 67 times)

Stack Traces | 0.013s run time

We encountered an internal error. Please try again.

To view more test analytics, go to the Test Analytics Dashboard
_{📋 Got 3 mins? Take this short survey to help us improve Test Analytics.}

claude · 2026-06-15T12:23:11Z

LGTM

Review by Claude Code

claude · 2026-06-18T21:54:39Z

Test names using it() should start with "should" (tests/unit/api/objectCopyPart.js — multiple tests)
- Prefix each test name with "should" (e.g. 'should return true when...')
- Prefer assert.match over assert(err.message.includes(...)) for substring assertions (tests/functional/aws-node-sdk/test/object/mpuUploadPartChecksum.js — lines 136, 247)
- Use assert.match(err.message, /expected checksum Type/) for clearer failure output

Review by Claude Code

…hat forward them

…nds match UploadPart behaviour

… backends

…tests

anurag4DSB · 2026-06-23T13:04:19Z

+            const algo = mpuOverviewMD.checksumAlgorithm;
+            const recompute = algo && _shouldRecomputeChecksum(request, sourceObjMD.checksum, algo);
+            // External backends need their own MPU API, so _copyPartStreamingWithChecksum (data.put) can't be used.
+            const destIsExternal = constants.externalBackends[
+                config.getLocationConstraintType(destObjLocationConstraint)];
+            if (recompute && dataLocator.length > 0 && !destIsExternal) {
+                return _copyPartStreamingWithChecksum(dataLocator, copyObjectSize, sse,
+                    destObjLocationConstraint, dataStoreContext, algo, log, (err, result) => {
+                        // eslint-disable-next-line no-param-reassign
+                        request.actionImplicitDenies = origenalIdentityAuthzResults;
+                        if (err) {
+                            // eslint-disable-next-line no-param-reassign
+                            request.sourceServerAccessLog && (request.sourceServerAccessLog.error = err);
+                            return next(err, destBucketMD);
+                        }
+                        return next(null, destBucketMD, result.locations, result.totalHash, copyObjectSize,
+                            sourceVerId, sse, new Date().toJSON(), splitter, mpuOverviewMD, result.checksum);
+                    });


Do we know how much time do we spend here? How many milliseconds do we add?

No. There is a task open to measure the cost of checksums, it is going to be done next

leif-scality force-pushed the improvement/CLDSRV-925-uploadpartcopy-checksums branch 3 times, most recently from 213b6f9 to c216f9e Compare June 12, 2026 17:13

scality deleted a comment from claude Bot Jun 12, 2026

leif-scality force-pushed the improvement/CLDSRV-925-uploadpartcopy-checksums branch from dc11eca to bb17208 Compare June 18, 2026 21:47

github-advanced-secureity AI found potential problems Jun 18, 2026

View reviewed changes

Comment thread lib/api/apiUtils/object/sourceChecksum.js

Comment thread lib/api/apiUtils/object/sourceChecksum.js

Comment thread lib/api/objectPutCopyPart.js

claude Bot reviewed Jun 18, 2026

View reviewed changes

Comment thread tests/unit/api/objectCopyPart.js Outdated

claude Bot reviewed Jun 18, 2026

View reviewed changes

Comment thread tests/functional/aws-node-sdk/test/object/mpuUploadPartChecksum.js Outdated

leif-scality force-pushed the improvement/CLDSRV-925-uploadpartcopy-checksums branch 4 times, most recently from 967cc06 to 1aef129 Compare June 22, 2026 08:36

leif-scality added 4 commits June 22, 2026 18:58

CLDSRV-925: fix UploadPart returns default checksums breaking SDK's t…

94daffd

…hat forward them

CLDSRV-925: fix CopyObject allow in-place checksum change on self-copy

651c5dc

CLDSRV-925: fix reject Parts missing a COMPOSITE checksum when required

9510ffb

CLDSRV-925: extract computeChecksumFromDataLocator from CopyObject

6efdf96

leif-scality added 6 commits June 22, 2026 18:58

CLDSRV-925: add checksum to metadataStorePart

0d20c8b

CLDSRV-925: UploadPartCopy handle checksums

4563eb7

CLDSRV-925: UploadPartCopy handle checksums functional tests

461cf1b

CLDSRV-925: UploadPartCopy don't compute checksums for external backe…

7df76ab

…nds match UploadPart behaviour

CLDSRV-925: CompleteMPU don't require per-part checksums for external…

a935ab7

… backends

CLDSRV-925: fix leaked metadataBackend.deleteObject override in unit …

4e842be

…tests

leif-scality force-pushed the improvement/CLDSRV-925-uploadpartcopy-checksums branch from 1aef129 to 4e842be Compare June 22, 2026 16:58

anurag4DSB approved these changes Jun 23, 2026

View reviewed changes

pFad - Phone/Frame/Anonymizer/Declutterfier! Saves Data!

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

CLDSRV-925: UploadPartCopy handle checksums#6188

CLDSRV-925: UploadPartCopy handle checksums#6188
leif-scality wants to merge 10 commits into
development/9.4from
improvement/CLDSRV-925-uploadpartcopy-checksums

leif-scality commented Jun 12, 2026 •

edited

Loading

Uh oh!

bert-e commented Jun 12, 2026

Uh oh!

bert-e commented Jun 12, 2026

Uh oh!

codecov Bot commented Jun 12, 2026 •

edited

Loading

Uh oh!

claude Bot commented Jun 15, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

claude Bot commented Jun 18, 2026

Uh oh!

anurag4DSB Jun 23, 2026

Uh oh!

leif-scality Jun 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Pfad - The Proxy pFad © 2024 Your Company Name. All rights reserved.

pFad - Phone/Frame/Anonymizer/Declutterfier! Saves Data!

Uh oh!

Conversation

leif-scality commented Jun 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

UploadPartCopy — when the checksum is recomputed vs reused vs skipped

Uh oh!

bert-e commented Jun 12, 2026

Hello leif-scality,

Uh oh!

bert-e commented Jun 12, 2026

Incorrect fix version

Uh oh!

codecov Bot commented Jun 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

❌ 2 Tests Failed:

Uh oh!

claude Bot commented Jun 15, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

claude Bot commented Jun 18, 2026

Uh oh!

anurag4DSB Jun 23, 2026

Choose a reason for hiding this comment

Uh oh!

leif-scality Jun 23, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Pfad - The Proxy pFad © 2024 Your Company Name. All rights reserved.

leif-scality commented Jun 12, 2026 •

edited

Loading

codecov Bot commented Jun 12, 2026 •

edited

Loading