Skip to content

S3UTILS-226 MPU orphan cleanup script#380

Open
jonathan-gramain wants to merge 2 commits intodevelopment/1.16from
improvement/S3UTILS-226-cleanup-mpu-orphans
Open

S3UTILS-226 MPU orphan cleanup script#380
jonathan-gramain wants to merge 2 commits intodevelopment/1.16from
improvement/S3UTILS-226-cleanup-mpu-orphans

Conversation

@jonathan-gramain
Copy link
Copy Markdown
Contributor

Implement cleanupMpuOrphans.js to delete orphaned parts and sproxyd keys.

Orphaned parts are part metadata which do not correspond to an overview key with the same upload ID.

Sproxyd keys are only removed if no completed MPU object has a matching upload ID in the same bucket.

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 13, 2026

Codecov Report

❌ Patch coverage is 61.67401% with 87 lines in your changes missing coverage. Please review.
✅ Project coverage is 45.23%. Comparing base (07cb65f) to head (bb4d32c).

Files with missing lines Patch % Lines
cleanupMpuOrphans.js 61.67% 86 Missing and 1 partial ⚠️
Additional details and impacted files
@@                 Coverage Diff                  @@
##           development/1.16     #380      +/-   ##
====================================================
+ Coverage             44.59%   45.23%   +0.64%     
====================================================
  Files                    84       85       +1     
  Lines                  5819     6046     +227     
  Branches               1237     1282      +45     
====================================================
+ Hits                   2595     2735     +140     
- Misses                 3179     3265      +86     
- Partials                 45       46       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@jonathan-gramain jonathan-gramain force-pushed the improvement/S3UTILS-225-list-bucketd-versions-full-metadata branch from 8ab54e6 to 07cb65f Compare March 25, 2026 18:14
Implement cleanupMpuOrphans.js to delete orphaned parts and sproxyd
keys.

Orphaned parts are part metadata which do not correspond to an
overview key with the same upload ID.

Sproxyd keys are only removed if no completed MPU object has a
matching upload ID in the same bucket.
@jonathan-gramain jonathan-gramain force-pushed the improvement/S3UTILS-226-cleanup-mpu-orphans branch from 3386bfa to e3f5b12 Compare March 25, 2026 18:14
Base automatically changed from improvement/S3UTILS-225-list-bucketd-versions-full-metadata to development/1.16 March 25, 2026 18:42
@bert-e
Copy link
Copy Markdown
Contributor

bert-e commented Mar 25, 2026

Hello jonathan-gramain,

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
Copy link
Copy Markdown
Contributor

bert-e commented Mar 25, 2026

Incorrect fix version

The Fix Version/s in issue S3UTILS-226 contains:

  • 1.17.5

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

  • 1.16.6

  • 1.17.6

  • 1.18.0

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

@jonathan-gramain
Copy link
Copy Markdown
Contributor Author

ping

@bert-e
Copy link
Copy Markdown
Contributor

bert-e commented Mar 25, 2026

Incorrect fix version

The Fix Version/s in issue S3UTILS-226 contains:

  • 1.16.6

  • 1.17.5

  • 1.18.0

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

  • 1.16.6

  • 1.17.6

  • 1.18.0

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

Add context of the script, its principle, and the main logs output
@jonathan-gramain jonathan-gramain force-pushed the improvement/S3UTILS-226-cleanup-mpu-orphans branch from 0458ae8 to bb4d32c Compare March 25, 2026 22:30
Copy link
Copy Markdown

@arnaudlangloisscality arnaudlangloisscality left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Copy link
Copy Markdown

@scality-fno scality-fno left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Strictly from a doc perspective.

I'm ok with the documentation. Some clarification could be beneficial. Not a blocker, though.

Comment on lines +473 to +495
log.info('starting discovery phase: scanning MPU shadow bucket', { bucket, shadowBucket });

status.bucket = bucket;
status.phase = 'discovery';
status.orphanedUploadIds = 0;
status.versionsScanned = 0;
status.orphanPartsDeleted = 0;
status.sproxydKeysDeleted = 0;

const orphanMap = await buildOrphanMap(
BUCKETD_HOSTPORT,
shadowBucket,
{ pageSize: LISTING_PAGE_SIZE, retry: RETRY_PARAMS },
);
const orphanCount = Object.keys(orphanMap).length;
status.orphanedUploadIds = orphanCount;
logProgress('discovery phase complete');
if (orphanCount === 0) {
return;
}

for (const [uploadId, info] of Object.entries(orphanMap)) {
log.info('orphaned MPU found', {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't know if the Usage section is meant to describe the exact sequence of the log messages, but if it is, the code contradicts it: the "orphaned MPU found" message comes last. Is this intentional?

Comment on lines +18 to +28
const VERBOSE = process.env.VERBOSE === '1';
const TRACE = process.env.TRACE === '1';

let logLevel;
if (TRACE) {
logLevel = 'trace';
} else if (VERBOSE) {
logLevel = 'debug';
} else {
logLevel = 'info';
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick: (Claude) Those three entries (deleted orphaned sproxyd key, not deleting sproxyd key used by completed MPU, deleted orphaned part metadata) are logged at debug level. They show up when either VERBOSE=1 or TRACE=1. Labelling them [VERBOSE ONLY] could confuse someone who set TRACE=1 and wonders why they're seeing those lines. [VERBOSE or TRACE] or just [requires VERBOSE=1 or TRACE=1] would be more accurate.

Comment on lines +66 to +69
- Cleanup phase: scans the bucket with a versioned listing,
matches any completed MPU with their orphaned counterpart to
detect used sproxyd keys, and only deletes the unused ones along
with the orphaned metadata
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doc clarity: Claude: That's accurate for the case where the orphaned upload ID matches a completed object. But the code also handles the complementary case: orphaned entries that don't match any completed version in the bucket are deleted entirely (all their sproxyd keys, unconditionally).

Suggested change
- Cleanup phase: scans the bucket with a versioned listing,
matches any completed MPU with their orphaned counterpart to
detect used sproxyd keys, and only deletes the unused ones along
with the orphaned metadata
- Cleanup phase: scans the bucket with a versioned listing,
matches any completed MPU with their orphaned counterpart to
detect used sproxyd keys, and only deletes the unused ones along
with the orphaned metadata. Any orphaned upload IDs that were not
matched to any completed object version are also deleted
unconditionally.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants