Details

Reviewers

michal
varun
• jon

Commits

rCOMM73401a024533: [blob] Delete S3 objects during cleanup

Summary

Final part of blob cleanup logic - added deletion of S3 objects.

Depends on D9352.

Test Plan

Locally, uploaded a few items to S3 with object names hashA, hashB.
Created blob item rows in DDB: one with 'unchecked' attribute (hashA) and the other without (hashB).
No holders were given.
Ran the cleanup: only hashA file was deleted from S3.

Diff Detail

Repository

rCOMM Comm

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

bartek created this revision.Oct 4 2023, 4:10 AM

bartek held this revision as a draft.

Herald added a reviewer: • jon. · View Herald TranscriptOct 4 2023, 4:10 AM

Herald added subscribers: will, tomek, ashoat. · View Herald Transcript

Harbormaster completed remote builds in B22964: Diff 31628.Oct 4 2023, 4:28 AM

bartek published this revision for review.Oct 4 2023, 6:49 AM

Rebase

bartek added a child revision: D9413: [blob] Add CLI option to run cleanup task.Oct 9 2023, 5:38 AM

Harbormaster completed remote builds in B23087: Diff 31789.Oct 9 2023, 6:01 AM

If I understand correctly this will try to remove S3 blobs that might not exist (for holders can exist without having a blob uploaded and the logic will run for them too). Is this expected?

services/blob/src/service.rs
304 ↗	(On Diff #31789)	`orphans` could be potentially changed to be a `HashMap<String /blob hash/, Vec<String /blob holder/>>`. Or maybe even iterate directly over `unchecked_items`?

In D9355#276898, @michal wrote:

If I understand correctly this will try to remove S3 blobs that might not exist (for holders can exist without having a blob uploaded and the logic will run for them too). Is this expected?

This is expected. It causes no harm. Anyway, this is very unlikely scenario, if not impossible, because a DDB row is created after a successful S3 upload.