CLDSRV-925: UploadPartCopy handle checksums by leif-scality · Pull Request #6188 · scality/cloudserver

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

❌ 1 Tests Failed:

Tests completed	Failed	Passed	Skipped
9617	1	9616	0

View the full list of 1 ❄️ 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 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 94 times)

Stack Traces | 0.018s 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

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 = originalIdentityAuthzResults;
+                        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

dvasilas · 2026-06-29T08:20:33Z

        sourceIsDestination &&
        whichMetadata === 'COPY' &&
        Object.keys(overrideMetadata).length === 0 &&
+        !headers['x-amz-checksum-algorithm'] &&


This means we will do the copy even if the source object already has the requested checksum (and we requested the same algorithm, so no change in the checksum), right?

Is this what AWS does?

Should we instead use _shouldRecomputeChecksum?

The original guard change was wrong. I checked AWS on a bucket with Bucket Keys turned on, where every self-copy looks like an encryption change so they all succeed; on a normal bucket AWS rejects a COPY self-copy that only sets a checksum and needs MetadataDirective=REPLACE instead.

The commit was changed and I only added a unit test to verify we follow the AWS behaviour

…hat forward them

…m change (match AWS)

…nds match UploadPart behaviour

… backends

…tests

github-advanced-security

CodeQL found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.

leif-scality · 2026-07-01T07:49:26Z

/approve

bert-e · 2026-07-01T07:49:35Z

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.

The following options are set: approve

leif-scality · 2026-07-01T07:53:33Z

ping

bert-e · 2026-07-01T07:53:45Z

I have successfully merged the changeset of this pull request
into targetted development branches:

✔️ development/9.4

The following branches have NOT changed:

development/7.10
development/7.4
development/7.70
development/8.8
development/9.0
development/9.1
development/9.2
development/9.3

This pull request did not target the following hotfix branch(es) so they
were left untouched:

hotfix/7.10.8
hotfix/7.4.8
hotfix/7.10.1
hotfix/7.10.3
hotfix/7.4.1
hotfix/7.4.6
hotfix/7.70.73
hotfix/9.0.7
hotfix/6.4.7
hotfix/7.2.0
hotfix/9.2.36
hotfix/7.70.11
hotfix/7.10.15
hotfix/7.10.49
hotfix/7.70.21
hotfix/7.9.0
hotfix/7.4.9
hotfix/8.8.45
hotfix/9.0.32
hotfix/7.10.27
hotfix/7.4.3
hotfix/7.7.0
hotfix/7.4.10
hotfix/7.8.0
hotfix/9.2.24
hotfix/7.4.2
hotfix/7.6.0
hotfix/7.10.0
hotfix/7.10.2
hotfix/7.4.5
hotfix/7.4.0
hotfix/7.70.51
hotfix/7.10.28
hotfix/7.4.7
hotfix/7.10.30
hotfix/7.10.4
hotfix/7.4.4
hotfix/7.70.45

Please check the status of the associated issue CLDSRV-925.

Goodbye leif-scality.

The following options are set: approve

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-security 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 Fixed

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 5 times, most recently from 1aef129 to 4e842be Compare June 22, 2026 16:58

anurag4DSB approved these changes Jun 23, 2026

View reviewed changes

leif-scality force-pushed the improvement/CLDSRV-925-uploadpartcopy-checksums branch 2 times, most recently from 44c9f14 to cd1d2ad Compare June 26, 2026 22:22

dvasilas reviewed Jun 29, 2026

View reviewed changes

leif-scality added 10 commits June 30, 2026 12:12

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

1295a6d

…hat forward them

CLDSRV-925: CopyObject require REPLACE directive for in-place checksu…

1d4b4d4

…m change (match AWS)

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

a3b92e5

CLDSRV-925: extract computeChecksumFromDataLocator from CopyObject

cc1e1e5

CLDSRV-925: add checksum to metadataStorePart

01317ed

CLDSRV-925: UploadPartCopy handle checksums

9725c85

CLDSRV-925: UploadPartCopy handle checksums functional tests

981b66e

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

879d313

…nds match UploadPart behaviour

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

e89bc47

… backends

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

5cd6b29

…tests

leif-scality force-pushed the improvement/CLDSRV-925-uploadpartcopy-checksums branch from cd1d2ad to 5cd6b29 Compare June 30, 2026 10:12

claude Bot reviewed Jun 30, 2026

View reviewed changes

Comment thread lib/api/objectPutPart.js Outdated

dvasilas approved these changes Jun 30, 2026

View reviewed changes

CLDSRV-925: prettier format

4334419

github-advanced-security AI found potential problems Jun 30, 2026

View reviewed changes

bert-e merged commit 4334419 into development/9.4 Jul 1, 2026
60 of 61 checks passed

bert-e deleted the improvement/CLDSRV-925-uploadpartcopy-checksums branch July 1, 2026 07:53

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!

❌ 1 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!

Uh oh!

Uh oh!

dvasilas Jun 29, 2026

Choose a reason for hiding this comment

Uh oh!

leif-scality Jun 30, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-advanced-security AI left a comment

Choose a reason for hiding this comment

Uh oh!

leif-scality commented Jul 1, 2026

Uh oh!

bert-e commented Jul 1, 2026

Incorrect fix version

Uh oh!

leif-scality commented Jul 1, 2026

Uh oh!

bert-e commented Jul 1, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

leif-scality commented Jun 12, 2026 •

edited

Loading

codecov Bot commented Jun 12, 2026 •

edited

Loading