Skip to content

feat: Implement MultipartUploadBackend on TieredStorage#458

Merged
lcian merged 11 commits into
mainfrom
lcian/feat/multipart-upload-tiered
May 29, 2026
Merged

feat: Implement MultipartUploadBackend on TieredStorage#458
lcian merged 11 commits into
mainfrom
lcian/feat/multipart-upload-tiered

Conversation

@lcian
Copy link
Copy Markdown
Member

@lcian lcian commented May 4, 2026

Last implementation of MultipartUploadBackend in objectstore-service.
Now TieredStorage requires a MultipartUploadBackend in its long_term slot.
All methods just pretty much forward to long_term, with the exception of complete_multipart which needs to use the Changelog for consistency and deal with the tombstones, with logic very similar to put_object except that we need an additional metadata read call.

Close FS-339

@lcian lcian requested a review from a team as a code owner May 4, 2026 15:36
@lcian lcian changed the title feat(gcs): implement MultipartUploadBackend using GCS XML API feat: Implement MultipartUploadBackend on TieredStorage May 4, 2026
@lcian lcian changed the base branch from main to lcian/feat/multipart-upload-other-backends May 4, 2026 15:37
@codecov

This comment has been minimized.

Comment thread objectstore-service/src/backend/tiered.rs Outdated
@lcian

This comment was marked as outdated.

@lcian lcian marked this pull request as draft May 4, 2026 15:50
@linear-code
Copy link
Copy Markdown

linear-code Bot commented May 5, 2026

Comment thread objectstore-service/src/backend/tiered.rs
Comment thread objectstore-service/src/backend/tiered.rs
@lcian lcian marked this pull request as ready for review May 7, 2026 08:54
@lcian

This comment was marked as off-topic.

@lcian lcian requested a review from matt-codecov May 7, 2026 08:55
Comment thread objectstore-service/src/backend/tiered.rs Outdated
Comment thread objectstore-service/src/backend/tiered.rs Outdated
Comment thread objectstore-service/src/backend/tiered.rs Outdated
Copy link
Copy Markdown
Contributor

@matt-codecov matt-codecov left a comment

Choose a reason for hiding this comment

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

stamp w comments

Comment thread objectstore-service/src/backend/changelog.rs
Comment thread objectstore-service/src/backend/mod.rs
Comment thread objectstore-service/src/backend/tiered.rs Outdated
Comment thread objectstore-service/src/backend/tiered.rs Outdated
@lcian lcian force-pushed the lcian/feat/multipart-upload-other-backends branch from e8a193d to 803c658 Compare May 12, 2026 10:43
Base automatically changed from lcian/feat/multipart-upload-other-backends to main May 12, 2026 11:22
lcian added 8 commits May 12, 2026 13:30
Move ChangeGuard creation before get_metadata so that if the metadata
read fails after complete_multipart succeeds, the guard cleans up the
assembled blob. Previously, Manual-expiration objects could be orphaned
permanently. Also fix redundant rustdoc link.
@lcian lcian force-pushed the lcian/feat/multipart-upload-tiered branch from 409be20 to d6eb979 Compare May 12, 2026 11:32
Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

There are 2 total unresolved issues (including 1 from previous review).

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit d6eb979. Configure here.

Comment thread objectstore-service/src/backend/tiered.rs Outdated
@lcian

This comment has been minimized.

@lcian
Copy link
Copy Markdown
Member Author

lcian commented May 27, 2026

Turning this into a draft to avoid getting confused until #480 has been reviewed.

@lcian lcian marked this pull request as draft May 27, 2026 10:15
…480)

⚠️ Stacked on #458

This addresses the issue being discussed in
#458 (comment)
by introducing a new `ChangePhase::Assembling` variant.
The corresponding `Change` carries a `cleanup_after` timestamp, allowing
the caller to defer cleanup.
In practice, we use this in `TieredStorage::complete_multipart` with an
arbitrary 24 hours deadline (should we change this?), during which the
user can freely retry completing the upload.

Note that this `ChangePhase` is special, as it behaves like
`ChangePhase::complete` in-process:
- It does nothing on `Drop`, as we assume that the deadline hasn't
passed yet.
- The corresponding `ChangeGuard` for it also doesn't spawn any task on
`Drop`, due to the same assumption.

As a consequence, this depends on a persistent implementation of the
`ChangeLog` to actually clean anything up, or at least on a task that
periodically scans the in-memory `ChangeLog` for `Change`s to clean up,
both of which we don't have yet.
In production, we will eventually always use a persistent `ChangeLog`,
and other code paths exist that similarly require a persistent
`ChangeLog` to actually ensure that no LT orphans are created, so I
think it's fine for this change to also make that assumption.

Part of FS-339.
Will be squash-merged into
#458 once approved.
@lcian lcian marked this pull request as ready for review May 29, 2026 08:01
@lcian lcian merged commit 507b076 into main May 29, 2026
24 checks passed
@lcian lcian deleted the lcian/feat/multipart-upload-tiered branch May 29, 2026 08:13
lcian added a commit that referenced this pull request May 29, 2026
⚠️ Stacked on #458

Final part of the Multipart uploads implementation server-side, this
implements the endpoints on top of the Service.

Close FS-341
Close FS-359
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.

2 participants